Service updates don't always reconcile when service routing is used.

internal/provider/kubernetes/controller.go

@@ -24,6 +24,7 @@ import (
 	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
+	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
@@ -403,7 +404,8 @@ func (r *gatewayAPIReconciler) processBackendRefs(ctx context.Context, gwcResour
 		switch backendRefKind {
 		case resource.KindService:
 			service := new(corev1.Service)
-			err := r.client.Get(ctx, types.NamespacedName{Namespace: string(*backendRef.Namespace), Name: string(backendRef.Name)}, service)
+			nsName := types.NamespacedName{Namespace: string(*backendRef.Namespace), Name: string(backendRef.Name)}
+			err := r.client.Get(ctx, nsName, service)
 			if err != nil {
 				r.log.Error(err, "failed to get Service", "namespace", string(*backendRef.Namespace),
 					"name", string(backendRef.Name))
@@ -413,7 +415,9 @@ func (r *gatewayAPIReconciler) processBackendRefs(ctx context.Context, gwcResour
 				r.log.Info("added Service to resource tree", "namespace", string(*backendRef.Namespace),
 					"name", string(backendRef.Name))
 			}
-			endpointSliceLabelKey = discoveryv1.LabelServiceName
+			if r.hasRouteWithEndpointRouting(&nsName) {
+				endpointSliceLabelKey = discoveryv1.LabelServiceName
+			}
 
 		case resource.KindServiceImport:
 			serviceImport := new(mcsapiv1a1.ServiceImport)
@@ -1352,12 +1356,21 @@ func (r *gatewayAPIReconciler) watchResources(ctx context.Context, mgr manager.M
 		}
 	}
 
+	// composable predicate functions - service updates do not require a reconcile when the
+	// service is not referenced by any endpoint-routed backend and ClusterIP is unchanged.
+	skipServiceUpdatesWithoutEndpointRouting := predicate.TypedFuncs[*corev1.Service]{
+		UpdateFunc: func(e event.TypedUpdateEvent[*corev1.Service]) bool {
+			return r.validateServiceUpdateForReconcile(e.ObjectOld, e.ObjectNew)
+		},
+	}
+
 	// Watch Service CRUDs and process affected *Route objects.
-	servicePredicates := []predicate.TypedPredicate[*corev1.Service]{
+	servicePredicates := []predicate.TypedPredicate[*corev1.Service]{predicate.And[*corev1.Service](
+		skipServiceUpdatesWithoutEndpointRouting,
 		predicate.NewTypedPredicateFuncs[*corev1.Service](func(svc *corev1.Service) bool {
 			return r.validateServiceForReconcile(svc)
 		}),
-	}
+	)}
 	if r.namespaceLabel != nil {
 		servicePredicates = append(servicePredicates, predicate.NewTypedPredicateFuncs[*corev1.Service](func(svc *corev1.Service) bool {
 			return r.hasMatchingNamespaceLabels(svc)

internal/provider/kubernetes/predicates.go

@@ -280,6 +280,31 @@ func (r *gatewayAPIReconciler) isOIDCHMACSecret(nsName *types.NamespacedName) bo
 	return *nsName == oidcHMACSecret
 }
 
+// validateServiceUpdateForReconcile checks whether a Service update should trigger a reconcile.
+// Returns false when the backend does not have endpoint routing and the service of type clusterIP
+// does not have a new IP address.
+func (r *gatewayAPIReconciler) validateServiceUpdateForReconcile(oldSvc *corev1.Service, newSvc *corev1.Service) bool {
+	ctx := context.Background()
+	labels := newSvc.GetLabels()
+	// Check if the Service belongs to a Gateway
+	gtw := r.findOwningGateway(ctx, labels)
+	if gtw != nil {
+		return true
+	}
+	// Merged gateways will have only this label
+	gcName, ok := labels[gatewayapi.OwningGatewayClassLabel]
+	if ok && r.mergeGateways.Has(gcName) {
+		return true
+	}
+
+	if (newSvc.Spec.Type == corev1.ServiceTypeClusterIP) && (oldSvc.Spec.Type == corev1.ServiceTypeClusterIP) && (newSvc.Spec.ClusterIP == oldSvc.Spec.ClusterIP) {
+		nsName := utils.NamespacedName(newSvc)
+		return r.hasRouteWithEndpointRouting(&nsName)
+	}
+
+	return true
+}
+
 // validateServiceForReconcile tries finding the owning Gateway of the Service
 // if it exists, finds the Gateway's Deployment, and further updates the Gateway
 // status Ready condition. All Services are pushed for reconciliation.

internal/provider/kubernetes/predicates_test.go

@@ -28,6 +28,7 @@ import (
 	"github.com/envoyproxy/gateway/internal/logging"
 	"github.com/envoyproxy/gateway/internal/message"
 	"github.com/envoyproxy/gateway/internal/provider/kubernetes/test"
+	"github.com/envoyproxy/gateway/internal/utils"
 )
 
 // TestGatewayClassHasMatchingController tests the hasMatchingController
@@ -1265,3 +1266,265 @@ func TestValidateHTTPRouteFilerForReconcile(t *testing.T) {
 		})
 	}
 }
+
+func TestServiceHasRouteWithEndpointRouting(t *testing.T) {
+	sampleGateway := test.GetGateway(types.NamespacedName{Name: "scheduled-status-test"}, "test-gc", 8080)
+
+	ep := test.GetEnvoyProxy(types.NamespacedName{Name: "test-ep"}, false)
+	epWithServiceRouting := ep.DeepCopy()
+	routing := egv1a1.ServiceRoutingType
+	epWithServiceRouting.Spec.RoutingType = &routing
+
+	epRef := &test.GroupKindNamespacedName{
+		Group:     gwapiv1.Group(ep.GroupVersionKind().Group),
+		Kind:      gwapiv1.Kind(ep.GroupVersionKind().Kind),
+		Namespace: gwapiv1.Namespace(ep.Namespace),
+		Name:      gwapiv1.ObjectName(ep.Name),
+	}
+
+	service := test.GetService(types.NamespacedName{Name: "service"}, nil, nil)
+
+	testCases := []struct {
+		name    string
+		configs []client.Object
+		service client.Object
+		expect  bool
+	}{
+		{
+			name: "service routing _ no routes exist",
+			configs: []client.Object{
+				test.GetGatewayClass("test-gc", egv1a1.GatewayControllerName, epRef),
+				sampleGateway,
+				epWithServiceRouting,
+			},
+			service: service,
+			expect:  false,
+		},
+		{
+			name: "service routing _ httproute",
+			configs: []client.Object{
+				test.GetGatewayClass("test-gc", egv1a1.GatewayControllerName, epRef),
+				sampleGateway,
+				epWithServiceRouting,
+				test.GetHTTPRoute(types.NamespacedName{Name: "httproute-test"}, "scheduled-status-test", types.NamespacedName{Name: "service"}, 80, ""),
+			},
+			service: service,
+			expect:  false,
+		},
+		{
+			name: "endpoint routing _ no routes exist",
+			configs: []client.Object{
+				test.GetGatewayClass("test-gc", egv1a1.GatewayControllerName, epRef),
+				sampleGateway,
+				ep,
+			},
+			service: service,
+			expect:  false,
+		},
+		{
+			name: "endpoint routing _ httproute",
+			configs: []client.Object{
+				test.GetGatewayClass("test-gc", egv1a1.GatewayControllerName, epRef),
+				sampleGateway,
+				ep,
+				test.GetHTTPRoute(types.NamespacedName{Name: "httproute-test"}, "scheduled-status-test", types.NamespacedName{Name: "service"}, 80, ""),
+			},
+			service: service,
+			expect:  true,
+		},
+	}
+
+	// Create the reconciler.
+	logger := logging.DefaultLogger(egv1a1.LogLevelInfo)
+
+	r := gatewayAPIReconciler{
+		classController: egv1a1.GatewayControllerName,
+		log:             logger,
+	}
+
+	for _, tc := range testCases {
+		r.client = fakeclient.NewClientBuilder().
+			WithScheme(envoygateway.GetScheme()).
+			WithObjects(tc.configs...).
+			WithIndex(&gwapiv1.HTTPRoute{}, backendHTTPRouteIndex, backendHTTPRouteIndexFunc).
+			Build()
+		t.Run(tc.name, func(t *testing.T) {
+			nsName := utils.NamespacedName(tc.service)
+			res := r.hasRouteWithEndpointRouting(&nsName)
+			require.Equal(t, tc.expect, res)
+		})
+	}
+}
+
+func TestValidateServiceUpdateForReconcile(t *testing.T) {
+	sampleGateway := test.GetGateway(types.NamespacedName{Name: "scheduled-status-test"}, "test-gc", 8080)
+
+	ep := test.GetEnvoyProxy(types.NamespacedName{Name: "test-ep"}, false)
+	epWithServiceRouting := ep.DeepCopy()
+	routing := egv1a1.ServiceRoutingType
+	epWithServiceRouting.Spec.RoutingType = &routing
+
+	epRef := &test.GroupKindNamespacedName{
+		Group:     gwapiv1.Group(ep.GroupVersionKind().Group),
+		Kind:      gwapiv1.Kind(ep.GroupVersionKind().Kind),
+		Namespace: gwapiv1.Namespace(ep.Namespace),
+		Name:      gwapiv1.ObjectName(ep.Name),
+	}
+
+	oldClusterIP := test.GetService(types.NamespacedName{Name: "service"}, nil, nil)
+	oldClusterIP.Spec.ClusterIP = "1.2.3.4"
+	oldClusterIP.Spec.Type = corev1.ServiceTypeClusterIP
+	oldClusterIP.Spec.Selector = map[string]string{"app": "test"}
+	newClusterIPSelectorChange := oldClusterIP.DeepCopy()
+	newClusterIPSelectorChange.Spec.Selector = map[string]string{"app": "test2"}
+	newClusterIPChange := oldClusterIP.DeepCopy()
+	newClusterIPChange.Spec.ClusterIP = "4.3.2.1"
+	oldNodePort := oldClusterIP.DeepCopy()
+	oldNodePort.Spec.Type = corev1.ServiceTypeNodePort
+
+	testCases := []struct {
+		name       string
+		configs    []client.Object
+		serviceOld *corev1.Service
+		serviceNew *corev1.Service
+		expect     bool
+	}{
+		{
+			name: "service routing _ clusterIP svc _ no routes _ no change",
+			configs: []client.Object{
+				test.GetGatewayClass("test-gc", egv1a1.GatewayControllerName, epRef),
+				sampleGateway,
+				epWithServiceRouting,
+			},
+			serviceOld: oldClusterIP,
+			serviceNew: oldClusterIP,
+			expect:     false,
+		},
+		{
+			name: "service routing _ clusterIP svc _ httproute _ no change",
+			configs: []client.Object{
+				test.GetGatewayClass("test-gc", egv1a1.GatewayControllerName, epRef),
+				sampleGateway,
+				epWithServiceRouting,
+				test.GetHTTPRoute(types.NamespacedName{Name: "httproute-test"}, "scheduled-status-test", types.NamespacedName{Name: "service"}, 80, ""),
+			},
+			serviceOld: oldClusterIP,
+			serviceNew: oldClusterIP,
+			expect:     false,
+		},
+		{
+			name: "service routing _ nodeport svc _ no routes _ no change",
+			configs: []client.Object{
+				test.GetGatewayClass("test-gc", egv1a1.GatewayControllerName, epRef),
+				sampleGateway,
+				epWithServiceRouting,
+			},
+			serviceOld: oldNodePort,
+			serviceNew: oldNodePort,
+			expect:     true,
+		},
+		{
+			name: "service routing _ nodeport svc _ httproute _ no change",
+			configs: []client.Object{
+				test.GetGatewayClass("test-gc", egv1a1.GatewayControllerName, epRef),
+				sampleGateway,
+				epWithServiceRouting,
+				test.GetHTTPRoute(types.NamespacedName{Name: "httproute-test"}, "scheduled-status-test", types.NamespacedName{Name: "service"}, 80, ""),
+			},
+			serviceOld: oldNodePort,
+			serviceNew: oldNodePort,
+			expect:     true,
+		},
+		{
+			name: "service routing _ clusterIP svc _ httproute _ change selector",
+			configs: []client.Object{
+				test.GetGatewayClass("test-gc", egv1a1.GatewayControllerName, epRef),
+				sampleGateway,
+				epWithServiceRouting,
+				test.GetHTTPRoute(types.NamespacedName{Name: "httproute-test"}, "scheduled-status-test", types.NamespacedName{Name: "service"}, 80, ""),
+			},
+			serviceOld: oldClusterIP,
+			serviceNew: newClusterIPSelectorChange,
+			expect:     false,
+		},
+		{
+			name: "service routing _ clusterIP svc _ httproute _ change clusterIP",
+			configs: []client.Object{
+				test.GetGatewayClass("test-gc", egv1a1.GatewayControllerName, epRef),
+				sampleGateway,
+				epWithServiceRouting,
+				test.GetHTTPRoute(types.NamespacedName{Name: "httproute-test"}, "scheduled-status-test", types.NamespacedName{Name: "service"}, 80, ""),
+			},
+			serviceOld: oldClusterIP,
+			serviceNew: newClusterIPChange,
+			expect:     true,
+		},
+		{
+			name: "endpoint routing _ clusterIP svc _ no routes _ no change",
+			configs: []client.Object{
+				test.GetGatewayClass("test-gc", egv1a1.GatewayControllerName, epRef),
+				sampleGateway,
+				ep,
+			},
+			serviceOld: oldClusterIP,
+			serviceNew: oldClusterIP,
+			expect:     false,
+		},
+		{
+			name: "endpoint routing _ clusterIP svc _ httproute _ no change",
+			configs: []client.Object{
+				test.GetGatewayClass("test-gc", egv1a1.GatewayControllerName, epRef),
+				sampleGateway,
+				ep,
+				test.GetHTTPRoute(types.NamespacedName{Name: "httproute-test"}, "scheduled-status-test", types.NamespacedName{Name: "service"}, 80, ""),
+			},
+			serviceOld: oldClusterIP,
+			serviceNew: oldClusterIP,
+			expect:     true,
+		},
+		{
+			name: "endpoint routing _ clusterIP svc _ httproute _ change selector",
+			configs: []client.Object{
+				test.GetGatewayClass("test-gc", egv1a1.GatewayControllerName, epRef),
+				sampleGateway,
+				ep,
+				test.GetHTTPRoute(types.NamespacedName{Name: "httproute-test"}, "scheduled-status-test", types.NamespacedName{Name: "service"}, 80, ""),
+			},
+			serviceOld: oldClusterIP,
+			serviceNew: newClusterIPSelectorChange,
+			expect:     true,
+		},
+		{
+			name: "endpoint routing _ clusterIP svc _ httproute _ change clusterIP",
+			configs: []client.Object{
+				test.GetGatewayClass("test-gc", egv1a1.GatewayControllerName, epRef),
+				sampleGateway,
+				ep,
+				test.GetHTTPRoute(types.NamespacedName{Name: "httproute-test"}, "scheduled-status-test", types.NamespacedName{Name: "service"}, 80, ""),
+			},
+			serviceOld: oldClusterIP,
+			serviceNew: newClusterIPChange,
+			expect:     true,
+		},
+	}
+
+	// Create the reconciler.
+	logger := logging.DefaultLogger(egv1a1.LogLevelInfo)
+
+	r := gatewayAPIReconciler{
+		classController: egv1a1.GatewayControllerName,
+		log:             logger,
+	}
+
+	for _, tc := range testCases {
+		r.client = fakeclient.NewClientBuilder().
+			WithScheme(envoygateway.GetScheme()).
+			WithObjects(tc.configs...).
+			WithIndex(&gwapiv1.HTTPRoute{}, backendHTTPRouteIndex, backendHTTPRouteIndexFunc).
+			Build()
+		t.Run(tc.name, func(t *testing.T) {
+			res := r.validateServiceUpdateForReconcile(tc.serviceOld, tc.serviceNew)
+			require.Equal(t, tc.expect, res)
+		})
+	}
+}