Address code review; fix unnecessary updates

Author: sjberman

apis/v1alpha2/nginxproxy_types.go

@@ -411,7 +411,7 @@ type DaemonSetSpec struct {
 	Container ContainerSpec `json:"container,omitempty"`
 }
 
-// Pod defines Pod-specific fields.
+// PodSpec defines Pod-specific fields.
 type PodSpec struct {
 	// TerminationGracePeriodSeconds is the optional duration in seconds the pod needs to terminate gracefully.
 	// Value must be non-negative integer. The value zero indicates stop immediately via
@@ -454,7 +454,7 @@ type PodSpec struct {
 	TopologySpreadConstraints []corev1.TopologySpreadConstraint `json:"topologySpreadConstraints,omitempty"`
 }
 
-// Container defines container fields for the nginx container.
+// ContainerSpec defines container fields for the nginx container.
 type ContainerSpec struct {
 	// Image is the nginx image to use.
 	//
@@ -512,7 +512,7 @@ const (
 	PullIfNotPresent PullPolicy = PullPolicy(corev1.PullIfNotPresent)
 )
 
-// Service is the configuration for the nginx Service.
+// ServiceSpec is the configuration for the nginx Service.
 type ServiceSpec struct {
 	// ServiceType describes ingress method for the service.
 	//

internal/mode/static/handler.go

@@ -41,7 +41,7 @@ type eventHandlerConfig struct {
 	ctx context.Context
 	// nginxUpdater updates nginx configuration using the NGINX agent.
 	nginxUpdater agent.NginxUpdater
-	// provisioner handles provisioning and deprovisioning nginx resources.
+	// nginxProvisioner handles provisioning and deprovisioning nginx resources.
 	nginxProvisioner provisioner.Provisioner
 	// metricsCollector collects metrics for this controller.
 	metricsCollector handlerMetricsCollector

internal/mode/static/provisioner/provisioner.go

@@ -281,7 +281,9 @@ func (p *NginxProvisioner) RegisterGateway(
 	resourceName string,
 ) error {
 	gatewayNSName := client.ObjectKeyFromObject(gateway.Source)
-	p.store.registerResourceInGatewayConfig(gatewayNSName, gateway)
+	if updated := p.store.registerResourceInGatewayConfig(gatewayNSName, gateway); !updated {
+		return nil
+	}
 
 	if gateway.Valid {
 		if err := p.provisionNginx(ctx, resourceName, gateway.Source, gateway.EffectiveNginxProxy); err != nil {

internal/mode/static/provisioner/store.go

@@ -1,6 +1,7 @@
 package provisioner
 
 import (
+	"reflect"
 	"strings"
 	"sync"
 
@@ -62,7 +63,11 @@ func (s *store) getGateway(nsName types.NamespacedName) *gatewayv1.Gateway {
 	return s.gateways[nsName]
 }
 
-func (s *store) registerResourceInGatewayConfig(gatewayNSName types.NamespacedName, object interface{}) {
+// registerResourceInGatewayConfig adds or updates the provided resource in the tracking map.
+// If the object being updated is the Gateway, check if anything that we care about changed. This ensures that
+// we don't attempt to update nginx resources when the main event handler triggers this call with an unrelated event
+// (like a Route update) that shouldn't result in nginx resource changes.
+func (s *store) registerResourceInGatewayConfig(gatewayNSName types.NamespacedName, object interface{}) bool {
 	s.lock.Lock()
 	defer s.lock.Unlock()
 
@@ -73,8 +78,9 @@ func (s *store) registerResourceInGatewayConfig(gatewayNSName types.NamespacedNa
 				Gateway: obj,
 			}
 		} else {
+			changed := gatewayChanged(cfg.Gateway, obj)
 			cfg.Gateway = obj
-			s.nginxResources[gatewayNSName] = cfg
+			return changed
 		}
 	case *appsv1.Deployment:
 		if cfg, ok := s.nginxResources[gatewayNSName]; !ok {
@@ -83,7 +89,6 @@ func (s *store) registerResourceInGatewayConfig(gatewayNSName types.NamespacedNa
 			}
 		} else {
 			cfg.Deployment = obj
-			s.nginxResources[gatewayNSName] = cfg
 		}
 	case *corev1.Service:
 		if cfg, ok := s.nginxResources[gatewayNSName]; !ok {
@@ -92,7 +97,6 @@ func (s *store) registerResourceInGatewayConfig(gatewayNSName types.NamespacedNa
 			}
 		} else {
 			cfg.Service = obj
-			s.nginxResources[gatewayNSName] = cfg
 		}
 	case *corev1.ServiceAccount:
 		if cfg, ok := s.nginxResources[gatewayNSName]; !ok {
@@ -101,7 +105,6 @@ func (s *store) registerResourceInGatewayConfig(gatewayNSName types.NamespacedNa
 			}
 		} else {
 			cfg.ServiceAccount = obj
-			s.nginxResources[gatewayNSName] = cfg
 		}
 	case *corev1.ConfigMap:
 		if cfg, ok := s.nginxResources[gatewayNSName]; !ok {
@@ -117,13 +120,25 @@ func (s *store) registerResourceInGatewayConfig(gatewayNSName types.NamespacedNa
 		} else {
 			if strings.HasSuffix(obj.GetName(), nginxIncludesConfigMapNameSuffix) {
 				cfg.BootstrapConfigMap = obj
-				s.nginxResources[gatewayNSName] = cfg
 			} else if strings.HasSuffix(obj.GetName(), nginxAgentConfigMapNameSuffix) {
 				cfg.AgentConfigMap = obj
-				s.nginxResources[gatewayNSName] = cfg
 			}
 		}
 	}
+
+	return true
+}
+
+func gatewayChanged(original, updated *graph.Gateway) bool {
+	if original.Valid != updated.Valid {
+		return true
+	}
+
+	if !reflect.DeepEqual(original.Source, updated.Source) {
+		return true
+	}
+
+	return !reflect.DeepEqual(original.EffectiveNginxProxy, updated.EffectiveNginxProxy)
 }
 
 func (s *store) getNginxResourcesForGateway(nsName types.NamespacedName) *NginxResources {

site/content/reference/api.md

@@ -1995,7 +1995,7 @@ sigs.k8s.io/gateway-api/apis/v1alpha2.PolicyStatus
 <a href="#gateway.nginx.org/v1alpha2.DeploymentSpec">DeploymentSpec</a>)
 </p>
 <p>
-<p>Container defines container fields for the nginx container.</p>
+<p>ContainerSpec defines container fields for the nginx container.</p>
 </p>
 <table class="table table-bordered table-striped">
 <thead>
@@ -2809,7 +2809,7 @@ be unique across all targetRef entries in the ObservabilityPolicy.</p>
 <a href="#gateway.nginx.org/v1alpha2.DeploymentSpec">DeploymentSpec</a>)
 </p>
 <p>
-<p>Pod defines Pod-specific fields.</p>
+<p>PodSpec defines Pod-specific fields.</p>
 </p>
 <table class="table table-bordered table-striped">
 <thead>
@@ -3130,7 +3130,7 @@ IP address in the X-Forwarded-For HTTP header.
 <a href="#gateway.nginx.org/v1alpha2.KubernetesSpec">KubernetesSpec</a>)
 </p>
 <p>
-<p>Service is the configuration for the nginx Service.</p>
+<p>ServiceSpec is the configuration for the nginx Service.</p>
 </p>
 <table class="table table-bordered table-striped">
 <thead>