add more unit tests, update handler

Author: salonichf5

examples/cafe-example/cafe-routes.yaml

@@ -6,8 +6,6 @@ spec:
   parentRefs:
   - name: gateway
     sectionName: http
-  - name: gateway3
-    sectionName: http
   hostnames:
   - "cafe.example.com"
   rules:
@@ -27,8 +25,6 @@ spec:
   parentRefs:
   - name: gateway
     sectionName: http
-  - name: gateway3
-    sectionName: http
   hostnames:
   - "cafe.example.com"
   rules:

examples/cafe-example/gateway.yaml

@@ -9,27 +9,3 @@ spec:
     port: 80
     protocol: HTTP
     hostname: "*.example.com"
----
-apiVersion: gateway.networking.k8s.io/v1
-kind: Gateway
-metadata:
-  name: gateway2
-spec:
-  gatewayClassName: nginx
-  listeners:
-  - name: http
-    port: 80
-    protocol: HTTP
-    hostname: "*.example.com"
----
-apiVersion: gateway.networking.k8s.io/v1
-kind: Gateway
-metadata:
-  name: gateway3
-spec:
-  gatewayClassName: nginx
-  listeners:
-  - name: http
-    port: 80
-    protocol: HTTP
-    hostname: "*.example.com"

internal/mode/static/handler.go

@@ -294,59 +294,60 @@ func (h *eventHandlerImpl) waitForStatusUpdates(ctx context.Context) {
 			return
 		}
 
+		// TODO(sberman): once we support multiple Gateways, we'll have to get
+		// the correct Graph for the Deployment contained in the update message
 		gr := h.cfg.processor.GetLatestGraph()
 		if gr == nil {
 			continue
 		}
 
-		deploymentName := item.Deployment
-		gw := gr.Gateways[deploymentName]
-
 		var nginxReloadRes graph.NginxReloadResult
-		switch {
-		case item.Error != nil:
-			h.cfg.logger.Error(item.Error, "Failed to update NGINX configuration")
-			nginxReloadRes.Error = item.Error
-		case gw != nil:
-			h.cfg.logger.Info("NGINX configuration was successfully updated")
-		}
-		gr.LatestReloadResult[deploymentName] = nginxReloadRes
-
-		switch item.UpdateType {
-		case status.UpdateAll:
-			h.updateStatuses(ctx, gr, gw)
-		case status.UpdateGateway:
-			gwAddresses, err := getGatewayAddresses(
-				ctx,
-				h.cfg.k8sClient,
-				item.GatewayService,
-				gw,
-				h.cfg.gatewayClassName,
-			)
-			if err != nil {
-				msg := "error getting Gateway Service IP address"
-				h.cfg.logger.Error(err, msg)
-				h.cfg.eventRecorder.Eventf(
+		for _, gw := range gr.Gateways {
+			switch {
+			case item.Error != nil:
+				h.cfg.logger.Error(item.Error, "Failed to update NGINX configuration")
+				nginxReloadRes.Error = item.Error
+			case gw != nil:
+				h.cfg.logger.Info("NGINX configuration was successfully updated")
+			}
+			gw.LatestReloadResult = nginxReloadRes
+
+			switch item.UpdateType {
+			case status.UpdateAll:
+				h.updateStatuses(ctx, gr, gw)
+			case status.UpdateGateway:
+				gwAddresses, err := getGatewayAddresses(
+					ctx,
+					h.cfg.k8sClient,
 					item.GatewayService,
-					v1.EventTypeWarning,
-					"GetServiceIPFailed",
-					msg+": %s",
-					err.Error(),
+					gw,
+					h.cfg.gatewayClassName,
 				)
-				continue
+				if err != nil {
+					msg := "error getting Gateway Service IP address"
+					h.cfg.logger.Error(err, msg)
+					h.cfg.eventRecorder.Eventf(
+						item.GatewayService,
+						v1.EventTypeWarning,
+						"GetServiceIPFailed",
+						msg+": %s",
+						err.Error(),
+					)
+					continue
+				}
+
+				transitionTime := metav1.Now()
+
+				gatewayStatuses := status.PrepareGatewayRequests(
+					gw,
+					transitionTime,
+					gwAddresses,
+					gw.LatestReloadResult,
+				)
+				h.cfg.statusUpdater.UpdateGroup(ctx, groupGateways, gatewayStatuses...)
+			default:
+				panic(fmt.Sprintf("unknown event type %T", item.UpdateType))
 			}
-
-			transitionTime := metav1.Now()
-
-			gatewayStatuses := status.PrepareGatewayRequests(
-				gw,
-				transitionTime,
-				gwAddresses,
-				gr.LatestReloadResult[deploymentName],
-			)
-			h.cfg.statusUpdater.UpdateGroup(ctx, groupGateways, gatewayStatuses...)
-		default:
-			panic(fmt.Sprintf("unknown event type %T", item.UpdateType))
 		}
 	}
 }
@@ -377,7 +378,7 @@ func (h *eventHandlerImpl) updateStatuses(ctx context.Context, gr *graph.Graph,
 		gr.L4Routes,
 		gr.Routes,
 		transitionTime,
-		gr.LatestReloadResult[gw.DeploymentName],
+		gw.LatestReloadResult,
 		h.cfg.gatewayCtlrName,
 	)
 
@@ -408,7 +409,7 @@ func (h *eventHandlerImpl) updateStatuses(ctx context.Context, gr *graph.Graph,
 		gw,
 		transitionTime,
 		gwAddresses,
-		gr.LatestReloadResult[gw.DeploymentName],
+		gw.LatestReloadResult,
 	)
 	h.cfg.statusUpdater.UpdateGroup(ctx, groupGateways, gwReqs...)
 }

internal/mode/static/handler_test.go

@@ -381,7 +381,8 @@ var _ = Describe("eventHandler", func() {
 			}).Should(Equal(2))
 
 		gr := handler.cfg.processor.GetLatestGraph()
-		Expect(gr.LatestReloadResult[types.NamespacedName{}].Error.Error()).To(Equal("status error"))
+		gw := gr.Gateways[types.NamespacedName{}]
+		Expect(gw.LatestReloadResult.Error.Error()).To(Equal("status error"))
 	})
 
 	It("should update Gateway status when receiving a queue event", func() {

internal/mode/static/state/change_processor_test.go

@@ -1007,7 +1007,6 @@ var _ = Describe("ChangeProcessor", func() {
 							GatewayNsNames: map[types.NamespacedName]struct{}{{Namespace: "test", Name: "gateway-1"}: {}},
 						},
 					},
-					LatestReloadResult: map[types.NamespacedName]graph.NginxReloadResult{},
 				}
 
 				expGraph2 = &graph.Graph{
@@ -1134,7 +1133,6 @@ var _ = Describe("ChangeProcessor", func() {
 							GatewayNsNames: map[types.NamespacedName]struct{}{{Namespace: "test", Name: "gateway-1"}: {}},
 						},
 					},
-					LatestReloadResult: map[types.NamespacedName]graph.NginxReloadResult{},
 				}
 			})
 			When("no upsert has occurred", func() {

internal/mode/static/state/graph/gateway.go

@@ -14,6 +14,7 @@ import (
 
 // Gateway represents the winning Gateway resource.
 type Gateway struct {
+	LatestReloadResult  NginxReloadResult
 	Source              *v1.Gateway
 	NginxProxy          *NginxProxy
 	EffectiveNginxProxy *EffectiveNginxProxy

internal/mode/static/state/graph/graph.go

@@ -76,8 +76,6 @@ type Graph struct {
 	SnippetsFilters map[types.NamespacedName]*SnippetsFilter
 	// PlusSecrets holds the secrets related to NGINX Plus licensing.
 	PlusSecrets map[types.NamespacedName][]PlusSecretFile
-	// LatestReloadResult is holds latest result of applying config to nginx for all gateways.
-	LatestReloadResult map[types.NamespacedName]NginxReloadResult
 }
 
 // NginxReloadResult describes the result of an NGINX reload.
@@ -291,11 +289,6 @@ func BuildGraph(
 
 	setPlusSecretContent(state.Secrets, plusSecrets)
 
-	var latestReloadResult map[types.NamespacedName]NginxReloadResult
-	if gws != nil {
-		latestReloadResult = make(map[types.NamespacedName]NginxReloadResult, len(gws))
-	}
-
 	g := &Graph{
 		GatewayClass:               gc,
 		Gateways:                   gws,
@@ -311,7 +304,6 @@ func BuildGraph(
 		NGFPolicies:                processedPolicies,
 		SnippetsFilters:            processedSnippetsFilters,
 		PlusSecrets:                plusSecrets,
-		LatestReloadResult:         latestReloadResult,
 	}
 
 	g.attachPolicies(controllerName)

internal/mode/static/state/graph/route_common.go

@@ -399,7 +399,8 @@ func getListenerHostPortMap(listeners []*Listener, gw *Gateway) map[string]hostP
 		Namespace: gw.Source.Namespace,
 	}
 	for _, l := range listeners {
-		listenerHostPortMap[l.Name] = hostPort{
+		key := fmt.Sprintf("%s,%s,%s", l.Name, gw.Source.Name, gw.Source.Namespace)
+		listenerHostPortMap[key] = hostPort{
 			hostname:  getHostname(l.Source.Hostname),
 			port:      l.Source.Port,
 			gwNsNames: gwNsNames,
@@ -447,6 +448,7 @@ func isolateHostnamesForParentRefs(parentRef []ParentRef, listenerHostnameMap ma
 			}
 			for _, h := range hostnames {
 				for lName, lHostPort := range listenerHostnameMap {
+					// skip comparison if not part of the same gateway
 					if lHostPort.gwNsNames != ref.Gateway {
 						continue
 					}
@@ -456,8 +458,12 @@ func isolateHostnamesForParentRefs(parentRef []ParentRef, listenerHostnameMap ma
 						continue
 					}
 
-					// for L7Routes, we compare the hostname, port and gatewayNamespace-gatewayName-listenerName combination
+					// for L7Routes, we compare the hostname, port and listenerName combination
 					// to identify if hostname needs to be isolated.
+					splitLName := strings.Split(lName, ",")
+					if len(splitLName) > 1 {
+						lName = splitLName[0]
+					}
 					if h == lHostPort.hostname && listenerName != lName {
 						// for L4Routes, we only compare the hostname and listener name combination
 						// because we do not allow l4Routes to attach to the same listener