From c6e432b54f5a4d105060bc568861c466f0e79014 Mon Sep 17 00:00:00 2001 From: janiussyafiq Date: Thu, 13 Aug 2026 23:51:37 +0800 Subject: [PATCH 1/5] fix: update publishService/statusAddress logic in gateway and ingress controller files (backport apache/apisix-ingress-controller#2732) Cherry-picked unmodified from apache/apisix-ingress-controller#2732 (originally apache/apisix-ingress-controller#2730), authored upstream by Hemanth Chebrolu. This fork was missing it, and apache/apisix-ingress-controller#2846 builds on it. --- docs/en/latest/reference/example.md | 9 +- internal/controller/gateway_controller.go | 33 ++- internal/controller/ingress_controller.go | 57 ++++- internal/controller/utils.go | 25 ++ test/e2e/gatewayapi/gateway.go | 186 ++++++++++++++ test/e2e/ingress/ingress.go | 297 ++++++++++++++++++++++ 6 files changed, 589 insertions(+), 18 deletions(-) diff --git a/docs/en/latest/reference/example.md b/docs/en/latest/reference/example.md index bee44bd8..72b356e9 100644 --- a/docs/en/latest/reference/example.md +++ b/docs/en/latest/reference/example.md @@ -1256,6 +1256,8 @@ spec: - 10.24.87.13 ``` +Each entry in `statusAddress` can be an IP address or a hostname. The controller automatically sets the address type on the Gateway status — `IPAddress` for valid IPs and `Hostname` for everything else. + @@ -1284,6 +1286,8 @@ spec: - 10.24.87.13 ``` +Each entry in `statusAddress` can be an IP address or a hostname. The controller automatically sets the `IP` field for valid IPs and the `Hostname` field for everything else in the Ingress load balancer status. + To configure the `publishService`: ```yaml @@ -1305,7 +1309,10 @@ spec: publishService: apisix-ee-3-gateway-gateway ``` -When using `publishService`, make sure your gateway Service is of `LoadBalancer` type the address can be populated. The controller will use the endpoint of this Service to update the status information of the Ingress resource. The format can be either `namespace/svc-name` or simply `svc-name` if the default namespace is correctly set. +When using `publishService`, the controller will use the endpoint of this Service to update the status information of the Ingress resource. The format can be either `namespace/svc-name` or simply `svc-name` if the default namespace is correctly set. + +- If the Service is of `LoadBalancer` type, the controller uses its external IP or hostname. +- If the Service is of `ClusterIP` type, the controller propagates the hostname from any Ingress resources that reference that Service. diff --git a/internal/controller/gateway_controller.go b/internal/controller/gateway_controller.go index 33282c62..bdde4d3d 100644 --- a/internal/controller/gateway_controller.go +++ b/internal/controller/gateway_controller.go @@ -21,6 +21,8 @@ import ( "context" "errors" "fmt" + "net" + "reflect" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" @@ -192,20 +194,26 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct msg: "gateway proxy not found", } } else { - if len(gateway.Status.Addresses) != len(gatewayProxy.Spec.StatusAddress) { - for _, addr := range gatewayProxy.Spec.StatusAddress { - if addr == "" { - continue - } - addrs = append(addrs, - gatewayv1.GatewayStatusAddress{ - Value: addr, - }, - ) + for _, addr := range gatewayProxy.Spec.StatusAddress { + if addr == "" { + continue } + addrType := gatewayv1.IPAddressType + if net.ParseIP(addr) == nil { + addrType = gatewayv1.HostnameAddressType + } + addrs = append(addrs, + gatewayv1.GatewayStatusAddress{ + Type: &addrType, + Value: addr, + }, + ) } } + // deduplicate in case statusAddress contains repeated values + addrs = deduplicateGatewayStatusAddresses(addrs) + listenerStatuses, err := getListenerStatus(ctx, r.Client, gateway) if err != nil { r.Log.Error(err, "failed to get listener status", "gateway", req.NamespacedName) @@ -221,8 +229,9 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct accepted := SetGatewayConditionAccepted(gateway, acceptStatus.status, acceptStatus.msg) programmed := SetGatewayConditionProgrammed(gateway, conditionProgrammedStatus, conditionProgrammedMsg) - if accepted || programmed || len(addrs) > 0 || len(listenerStatuses) > 0 { - if len(addrs) > 0 { + addressesChanged := !reflect.DeepEqual(gateway.Status.Addresses, addrs) + if accepted || programmed || addressesChanged || len(listenerStatuses) > 0 { + if addressesChanged { gateway.Status.Addresses = addrs } if len(listenerStatuses) > 0 { diff --git a/internal/controller/ingress_controller.go b/internal/controller/ingress_controller.go index 26c93fab..517be554 100644 --- a/internal/controller/ingress_controller.go +++ b/internal/controller/ingress_controller.go @@ -20,6 +20,7 @@ package controller import ( "context" "fmt" + "net" "reflect" "github.com/go-logr/logr" @@ -737,9 +738,13 @@ func (r *IngressReconciler) updateStatus(ctx context.Context, tctx *provider.Tra if addr == "" { continue } - loadBalancerStatus.Ingress = append(loadBalancerStatus.Ingress, networkingv1.IngressLoadBalancerIngress{ - IP: addr, - }) + lbIngress := networkingv1.IngressLoadBalancerIngress{} + if net.ParseIP(addr) != nil { + lbIngress.IP = addr + } else { + lbIngress.Hostname = addr + } + loadBalancerStatus.Ingress = append(loadBalancerStatus.Ingress, lbIngress) } } else { // 2. if the IngressStatusAddress is not configured, try to use the PublishService @@ -760,7 +765,8 @@ func (r *IngressReconciler) updateStatus(ctx context.Context, tctx *provider.Tra return fmt.Errorf("failed to get publish service %s: %w", publishService, err) } - if svc.Spec.Type == corev1.ServiceTypeLoadBalancer { + switch svc.Spec.Type { + case corev1.ServiceTypeLoadBalancer: // get the LoadBalancer IP and Hostname of the service for _, ip := range svc.Status.LoadBalancer.Ingress { if ip.IP != "" { @@ -774,12 +780,53 @@ func (r *IngressReconciler) updateStatus(ctx context.Context, tctx *provider.Tra }) } } + case corev1.ServiceTypeClusterIP: + // For ClusterIP services, propagate load balancer status from any other + // Ingress that lists this service as a backend (e.g. a cloud LB Ingress + // fronting the APISIX ClusterIP Service). Uses ServiceIndexRef, which + // indexes Ingresses by spec.rules[].http.paths[].backend.service.name. + ingressList := &networkingv1.IngressList{} + if err := r.List(ctx, ingressList, client.MatchingFields{ + indexer.ServiceIndexRef: indexer.GenIndexKey(namespace, name), + }); err != nil { + return fmt.Errorf("failed to list ingresses for ClusterIP service %s/%s: %w", namespace, name, err) + } + if len(ingressList.Items) == 0 { + r.Log.V(1).Info("no Ingress found with this ClusterIP service as a backend; status will not be propagated", + "service", namespace+"/"+name) + } + for _, ing := range ingressList.Items { + // Skip the current Ingress being reconciled to avoid a + // self-referential loop: updating its own status would trigger + // a new reconcile, which would collect its own (just-written) + // hostname again and potentially repeat indefinitely. + if ing.Namespace == ingress.Namespace && ing.Name == ingress.Name { + continue + } + for _, lb := range ing.Status.LoadBalancer.Ingress { + if lb.IP != "" { + loadBalancerStatus.Ingress = append(loadBalancerStatus.Ingress, networkingv1.IngressLoadBalancerIngress{ + IP: lb.IP, + }) + } + if lb.Hostname != "" { + loadBalancerStatus.Ingress = append(loadBalancerStatus.Ingress, networkingv1.IngressLoadBalancerIngress{ + Hostname: lb.Hostname, + }) + } + } + } } } } + // deduplicate load balancer ingress entries that may arise when multiple + // source Ingresses carry the same address (ClusterIP case) or when + // statusAddress contains repeated values. + loadBalancerStatus.Ingress = deduplicateLoadBalancerIngress(loadBalancerStatus.Ingress) + // update the load balancer status - if len(loadBalancerStatus.Ingress) > 0 && !reflect.DeepEqual(ingress.Status.LoadBalancer, loadBalancerStatus) { + if !reflect.DeepEqual(ingress.Status.LoadBalancer, loadBalancerStatus) { ingress.Status.LoadBalancer = loadBalancerStatus r.Updater.Update(status.Update{ NamespacedName: utils.NamespacedName(ingress), diff --git a/internal/controller/utils.go b/internal/controller/utils.go index 7623a53e..445da8ea 100644 --- a/internal/controller/utils.go +++ b/internal/controller/utils.go @@ -2137,3 +2137,28 @@ func ExtractIngressClass(obj client.Object) string { panic(fmt.Errorf("unhandled object type %T for extracting ingress class", obj)) } } + +// deduplicateLoadBalancerIngress removes duplicate IngressLoadBalancerIngress entries in-place, +// comparing by IP and Hostname (Ports are ignored for dedup purposes). +func deduplicateLoadBalancerIngress(entries []networkingv1.IngressLoadBalancerIngress) []networkingv1.IngressLoadBalancerIngress { + slices.SortFunc(entries, func(a, b networkingv1.IngressLoadBalancerIngress) int { + if c := strings.Compare(a.IP, b.IP); c != 0 { + return c + } + return strings.Compare(a.Hostname, b.Hostname) + }) + return slices.CompactFunc(entries, func(a, b networkingv1.IngressLoadBalancerIngress) bool { + return a.IP == b.IP && a.Hostname == b.Hostname + }) +} + +// deduplicateGatewayStatusAddresses removes duplicate GatewayStatusAddress entries in-place, +// comparing by Value field (AddressType is a pointer so cannot be used as map key). +func deduplicateGatewayStatusAddresses(addrs []gatewayv1.GatewayStatusAddress) []gatewayv1.GatewayStatusAddress { + slices.SortFunc(addrs, func(a, b gatewayv1.GatewayStatusAddress) int { + return strings.Compare(a.Value, b.Value) + }) + return slices.CompactFunc(addrs, func(a, b gatewayv1.GatewayStatusAddress) bool { + return a.Value == b.Value + }) +} diff --git a/test/e2e/gatewayapi/gateway.go b/test/e2e/gatewayapi/gateway.go index 212e6926..8d660fd6 100644 --- a/test/e2e/gatewayapi/gateway.go +++ b/test/e2e/gatewayapi/gateway.go @@ -645,4 +645,190 @@ spec: Expect(string(getListener.SupportedKinds[0].Kind)).To(Equal("UDPRoute"), "udp listener supported kind content") }) }) + + Context("Gateway Status Address", func() { + var gatewayProxyWithStatusAddressYaml = ` +apiVersion: apisix.apache.org/v1alpha1 +kind: GatewayProxy +metadata: + name: apisix-proxy-config + namespace: %s +spec: + statusAddress: + - %s + provider: + type: ControlPlane + controlPlane: + endpoints: + - %s + auth: + type: AdminKey + adminKey: + value: "%s" +` + var defaultGatewayClass = ` +apiVersion: gateway.networking.k8s.io/v1 +kind: GatewayClass +metadata: + name: %s +spec: + controllerName: "%s" +` + var defaultGateway = ` +apiVersion: gateway.networking.k8s.io/v1 +kind: Gateway +metadata: + name: %s +spec: + gatewayClassName: %s + listeners: + - name: http + protocol: HTTP + port: 80 + infrastructure: + parametersRef: + group: apisix.apache.org + kind: GatewayProxy + name: apisix-proxy-config +` + getGatewayAddresses := func(gatewayName string) ([]gatewayv1.GatewayStatusAddress, error) { + var gateway gatewayv1.Gateway + if err := s.GetKubeClient().Get(context.Background(), k8stypes.NamespacedName{ + Name: gatewayName, + Namespace: s.Namespace(), + }, &gateway); err != nil { + return nil, err + } + return gateway.Status.Addresses, nil + } + + assertGatewayAddress := func(gatewayName, expectedValue string, expectedType gatewayv1.AddressType) { + s.RetryAssertion(func() error { + addrs, err := getGatewayAddresses(gatewayName) + if err != nil { + return err + } + if len(addrs) == 0 { + return fmt.Errorf("expected at least 1 status address, got 0") + } + addr := addrs[0] + if addr.Value != expectedValue { + return fmt.Errorf("expected address value %s, got %s", expectedValue, addr.Value) + } + if addr.Type == nil { + return fmt.Errorf("expected address type to be set, got nil") + } + if *addr.Type != expectedType { + return fmt.Errorf("expected address type %s, got %s", expectedType, *addr.Type) + } + return nil + }).ShouldNot(HaveOccurred(), "check Gateway status address") + } + + createGatewayClassAndGateway := func(gatewayClassName, gatewayName string) { + By("create GatewayClass") + Expect(s.CreateResourceFromStringWithNamespace( + fmt.Sprintf(defaultGatewayClass, gatewayClassName, s.GetControllerName()), ""), + ).NotTo(HaveOccurred(), "creating GatewayClass") + + By("create Gateway") + Expect(s.CreateResourceFromStringWithNamespace( + fmt.Sprintf(defaultGateway, gatewayName, gatewayClassName), s.Namespace()), + ).NotTo(HaveOccurred(), "creating Gateway") + } + + checkGatewayStatusAddressType := func(addrValue string, expectedType gatewayv1.AddressType) { + gatewayClassName := s.Namespace() + + By("create GatewayProxy with statusAddress") + gatewayProxy := fmt.Sprintf(gatewayProxyWithStatusAddressYaml, + s.Namespace(), addrValue, s.Deployer.GetAdminEndpoint(), s.AdminKey()) + Expect(s.CreateResourceFromString(gatewayProxy)).NotTo(HaveOccurred(), "creating GatewayProxy") + + gatewayName := s.Namespace() + createGatewayClassAndGateway(gatewayClassName, gatewayName) + + By("check Gateway status address type") + assertGatewayAddress(gatewayName, addrValue, expectedType) + } + + It("sets IPAddress type when statusAddress is an IP", func() { + checkGatewayStatusAddressType("192.168.1.100", gatewayv1.IPAddressType) + }) + + It("sets Hostname type when statusAddress is a hostname", func() { + checkGatewayStatusAddressType("mygateway.example.com", gatewayv1.HostnameAddressType) + }) + + It("deduplicates repeated statusAddress entries", func() { + gatewayClassName := s.Namespace() + gatewayName := s.Namespace() + addr := "192.168.1.100" + + By("create GatewayProxy with the same IP listed twice in statusAddress") + gatewayProxy := fmt.Sprintf(` +apiVersion: apisix.apache.org/v1alpha1 +kind: GatewayProxy +metadata: + name: apisix-proxy-config + namespace: %s +spec: + statusAddress: + - %s + - %s + provider: + type: ControlPlane + controlPlane: + endpoints: + - %s + auth: + type: AdminKey + adminKey: + value: "%s" +`, s.Namespace(), addr, addr, s.Deployer.GetAdminEndpoint(), s.AdminKey()) + Expect(s.CreateResourceFromString(gatewayProxy)).NotTo(HaveOccurred(), "creating GatewayProxy") + + createGatewayClassAndGateway(gatewayClassName, gatewayName) + + By("verify only one address appears in Gateway status despite duplicate input") + s.RetryAssertion(func() error { + addrs, err := getGatewayAddresses(gatewayName) + if err != nil { + return err + } + if len(addrs) != 1 { + return fmt.Errorf("expected exactly 1 status address after dedup, got %d", len(addrs)) + } + if addrs[0].Value != addr { + return fmt.Errorf("expected address value %s, got %s", addr, addrs[0].Value) + } + return nil + }).ShouldNot(HaveOccurred(), "check Gateway status address deduplication") + }) + + It("updates status when statusAddress value changes without count change", func() { + gatewayClassName := s.Namespace() + gatewayName := s.Namespace() + initialAddr := "192.168.1.100" + updatedAddr := "updated.example.com" + + By("create GatewayProxy with initial statusAddress") + gatewayProxy := fmt.Sprintf(gatewayProxyWithStatusAddressYaml, + s.Namespace(), initialAddr, s.Deployer.GetAdminEndpoint(), s.AdminKey()) + Expect(s.CreateResourceFromString(gatewayProxy)).NotTo(HaveOccurred(), "creating GatewayProxy") + + createGatewayClassAndGateway(gatewayClassName, gatewayName) + + By("verify initial status address is set") + assertGatewayAddress(gatewayName, initialAddr, gatewayv1.IPAddressType) + + By("update GatewayProxy with different statusAddress (same count)") + updatedGatewayProxy := fmt.Sprintf(gatewayProxyWithStatusAddressYaml, + s.Namespace(), updatedAddr, s.Deployer.GetAdminEndpoint(), s.AdminKey()) + Expect(s.CreateResourceFromString(updatedGatewayProxy)).NotTo(HaveOccurred(), "updating GatewayProxy") + + By("verify status address is updated to new value and type") + assertGatewayAddress(gatewayName, updatedAddr, gatewayv1.HostnameAddressType) + }) + }) }) diff --git a/test/e2e/ingress/ingress.go b/test/e2e/ingress/ingress.go index f1493541..5dd132e9 100644 --- a/test/e2e/ingress/ingress.go +++ b/test/e2e/ingress/ingress.go @@ -30,6 +30,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/stretchr/testify/assert" + networkingv1 "k8s.io/api/networking/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" @@ -1285,4 +1286,300 @@ spec: Should(Equal("enabled")) }) }) + + Context("Ingress Status Address", func() { + var gatewayProxyWithPublishServiceYaml = ` +apiVersion: apisix.apache.org/v1alpha1 +kind: GatewayProxy +metadata: + name: apisix-proxy-config + namespace: %s +spec: + publishService: %s/%s + provider: + type: ControlPlane + controlPlane: + endpoints: + - %s + auth: + type: AdminKey + adminKey: + value: "%s" +` + var gatewayProxyWithStatusAddressYaml = ` +apiVersion: apisix.apache.org/v1alpha1 +kind: GatewayProxy +metadata: + name: apisix-proxy-config + namespace: %s +spec: + statusAddress: + - %s + provider: + type: ControlPlane + controlPlane: + endpoints: + - %s + auth: + type: AdminKey + adminKey: + value: "%s" +` + var ingressClassYaml = ` +apiVersion: networking.k8s.io/v1 +kind: IngressClass +metadata: + name: %s +spec: + controller: "%s" + parameters: + apiGroup: "apisix.apache.org" + kind: "GatewayProxy" + name: "apisix-proxy-config" + namespace: "%s" + scope: "Namespace" +` + var ingressYaml = ` +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: %s +spec: + ingressClassName: %s + rules: + - host: status.example.com + http: + paths: + - path: / + pathType: Prefix + backend: + service: + name: %s + port: + number: 80 +` + getIngressLBStatus := func(ingressName string) ([]networkingv1.IngressLoadBalancerIngress, error) { + ing := &networkingv1.Ingress{} + if err := s.K8sClient.Get(context.Background(), types.NamespacedName{ + Name: ingressName, + Namespace: s.Namespace(), + }, ing); err != nil { + return nil, err + } + return ing.Status.LoadBalancer.Ingress, nil + } + + createIngressClass := func(ingressClassName string) { + By("create IngressClass") + Expect(s.CreateResourceFromStringWithNamespace( + fmt.Sprintf(ingressClassYaml, ingressClassName, s.GetControllerName(), s.Namespace()), ""), + ).NotTo(HaveOccurred(), "creating IngressClass") + DeferCleanup(func() { + ingressClass := &networkingv1.IngressClass{} + if err := s.K8sClient.Get(context.Background(), types.NamespacedName{ + Name: ingressClassName, + }, ingressClass); err == nil { + _ = s.K8sClient.Delete(context.Background(), ingressClass) + } + }) + } + + checkIngressStatusAddress := func(addrValue, ingressSuffix, expectedIP, expectedHostname string) { + ingressClassName := s.Namespace() + + By("create GatewayProxy with statusAddress") + gatewayProxy := fmt.Sprintf(gatewayProxyWithStatusAddressYaml, + s.Namespace(), addrValue, s.Deployer.GetAdminEndpoint(), s.AdminKey()) + Expect(s.CreateResourceFromStringWithNamespace(gatewayProxy, s.Namespace())).NotTo(HaveOccurred(), "creating GatewayProxy") + + createIngressClass(ingressClassName) + + By("create Ingress") + ingressName := s.Namespace() + ingressSuffix + Expect(s.CreateResourceFromStringWithNamespace( + fmt.Sprintf(ingressYaml, ingressName, ingressClassName, "httpbin-service-e2e-test"), s.Namespace()), + ).NotTo(HaveOccurred(), "creating Ingress") + + By("check Ingress load balancer status") + s.RetryAssertion(func() error { + lbs, err := getIngressLBStatus(ingressName) + if err != nil { + return err + } + if len(lbs) == 0 { + return fmt.Errorf("expected at least 1 load balancer ingress, got 0") + } + if lbs[0].IP != expectedIP { + return fmt.Errorf("expected IP %s, got %s", expectedIP, lbs[0].IP) + } + if lbs[0].Hostname != expectedHostname { + return fmt.Errorf("expected Hostname %s, got %s", expectedHostname, lbs[0].Hostname) + } + return nil + }).ShouldNot(HaveOccurred(), "check Ingress load balancer status") + } + + It("sets IP field when statusAddress is an IP", func() { + checkIngressStatusAddress("192.168.1.200", "-ip", "192.168.1.200", "") + }) + + It("sets Hostname field when statusAddress is a hostname", func() { + checkIngressStatusAddress("myingress.example.com", "-host", "", "myingress.example.com") + }) + + It("deduplicates repeated statusAddress entries", func() { + addr := "192.168.1.200" + ingressClassName := s.Namespace() + + By("create GatewayProxy with the same IP listed twice in statusAddress") + gatewayProxy := fmt.Sprintf(` +apiVersion: apisix.apache.org/v1alpha1 +kind: GatewayProxy +metadata: + name: apisix-proxy-config + namespace: %s +spec: + statusAddress: + - %s + - %s + provider: + type: ControlPlane + controlPlane: + endpoints: + - %s + auth: + type: AdminKey + adminKey: + value: "%s" +`, s.Namespace(), addr, addr, s.Deployer.GetAdminEndpoint(), s.AdminKey()) + Expect(s.CreateResourceFromStringWithNamespace(gatewayProxy, s.Namespace())).NotTo(HaveOccurred(), "creating GatewayProxy") + + createIngressClass(ingressClassName) + + By("create Ingress") + ingressName := s.Namespace() + "-dedup" + Expect(s.CreateResourceFromStringWithNamespace( + fmt.Sprintf(ingressYaml, ingressName, ingressClassName, "httpbin-service-e2e-test"), s.Namespace()), + ).NotTo(HaveOccurred(), "creating Ingress") + + By("verify only one address appears in Ingress status despite duplicate input") + s.RetryAssertion(func() error { + lbs, err := getIngressLBStatus(ingressName) + if err != nil { + return err + } + if len(lbs) != 1 { + return fmt.Errorf("expected exactly 1 load balancer ingress after dedup, got %d", len(lbs)) + } + if lbs[0].IP != addr { + return fmt.Errorf("expected IP %s, got %s", addr, lbs[0].IP) + } + return nil + }).ShouldNot(HaveOccurred(), "check Ingress load balancer status deduplication") + }) + + It("propagates hostname from referencing Ingress when publishService is a ClusterIP", func() { + clusterIPSvcName := "apisix-clusterip-svc" + expectedHostname := "clusterip.example.com" + ingressClassName := s.Namespace() + + By("create ClusterIP service") + clusterIPSvc := fmt.Sprintf(` +apiVersion: v1 +kind: Service +metadata: + name: %s + namespace: %s +spec: + type: ClusterIP + selector: + app: httpbin + ports: + - port: 80 + targetPort: 80 +`, clusterIPSvcName, s.Namespace()) + Expect(s.CreateResourceFromStringWithNamespace(clusterIPSvc, s.Namespace())).NotTo(HaveOccurred(), "creating ClusterIP service") + + By("create source Ingress referencing the ClusterIP service") + sourceIngressName := s.Namespace() + "-source" + // The source Ingress intentionally has no ingressClassName — it simulates a + // cloud ALB Ingress managed by a different controller. Without ingressClassName + // our controller will not reconcile it and will not overwrite its status. + sourceIngressYaml := fmt.Sprintf(` +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: %s +spec: + rules: + - host: clusterip.example.com + http: + paths: + - path: / + pathType: Prefix + backend: + service: + name: %s + port: + number: 80 +`, sourceIngressName, clusterIPSvcName) + Expect(s.CreateResourceFromStringWithNamespace(sourceIngressYaml, s.Namespace())).NotTo(HaveOccurred(), "creating source Ingress") + + By("patch source Ingress status with hostname") + sourceIng := &networkingv1.Ingress{} + Expect(s.K8sClient.Get(context.Background(), types.NamespacedName{ + Name: sourceIngressName, Namespace: s.Namespace(), + }, sourceIng)).NotTo(HaveOccurred(), "getting source Ingress") + sourceIng.Status.LoadBalancer.Ingress = []networkingv1.IngressLoadBalancerIngress{ + {Hostname: expectedHostname}, + } + Expect(s.K8sClient.Status().Update(context.Background(), sourceIng)).NotTo(HaveOccurred(), "patching source Ingress status") + + By("create GatewayProxy with ClusterIP publishService") + gatewayProxy := fmt.Sprintf(gatewayProxyWithPublishServiceYaml, + s.Namespace(), s.Namespace(), clusterIPSvcName, s.Deployer.GetAdminEndpoint(), s.AdminKey()) + Expect(s.CreateResourceFromStringWithNamespace(gatewayProxy, s.Namespace())).NotTo(HaveOccurred(), "creating GatewayProxy") + + createIngressClass(ingressClassName) + + By("create main Ingress") + mainIngressName := s.Namespace() + "-main" + mainIngress := fmt.Sprintf(` +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: %s +spec: + ingressClassName: %s + rules: + - host: status.example.com + http: + paths: + - path: / + pathType: Prefix + backend: + service: + name: httpbin-service-e2e-test + port: + number: 80 +`, mainIngressName, ingressClassName) + Expect(s.CreateResourceFromStringWithNamespace(mainIngress, s.Namespace())).NotTo(HaveOccurred(), "creating main Ingress") + + By("check main Ingress status propagates hostname from source Ingress") + s.RetryAssertion(func() error { + lbs, err := getIngressLBStatus(mainIngressName) + if err != nil { + return err + } + if len(lbs) == 0 { + return fmt.Errorf("expected at least 1 load balancer ingress, got 0") + } + if lbs[0].Hostname != expectedHostname { + return fmt.Errorf("expected Hostname %s, got %s", expectedHostname, lbs[0].Hostname) + } + return nil + }).ShouldNot(HaveOccurred(), "check main Ingress load balancer status from ClusterIP") + }) + + }) }) From 8c0c3394cfd22ca59c24467ef56d908b94521bba Mon Sep 17 00:00:00 2001 From: janiussyafiq Date: Thu, 13 Aug 2026 11:39:36 +0800 Subject: [PATCH 2/5] fix: fall back to publishService for Gateway status addresses Gateway.status.addresses was only populated from GatewayProxy.spec.statusAddress. When spec.publishService is set instead, the Gateway published no address, even though the Ingress status path already falls back to the publish Service's LoadBalancer address. Extract shared publish-Service resolution helpers, make Gateway.status.addresses fall back to the LoadBalancer address of spec.publishService when statusAddress is empty (statusAddress still wins when both are set), and refactor the Ingress LoadBalancer status branch onto the same helpers. --- internal/controller/gateway_controller.go | 44 ++++- internal/controller/ingress_controller.go | 34 ++-- internal/controller/utils.go | 55 ++++-- .../controller/utils_publishservice_test.go | 41 ++++ internal/utils/k8s.go | 18 ++ test/e2e/gatewayapi/gateway.go | 179 ++++++++++++++++++ 6 files changed, 328 insertions(+), 43 deletions(-) create mode 100644 internal/controller/utils_publishservice_test.go diff --git a/internal/controller/gateway_controller.go b/internal/controller/gateway_controller.go index bdde4d3d..aaf599c9 100644 --- a/internal/controller/gateway_controller.go +++ b/internal/controller/gateway_controller.go @@ -194,10 +194,14 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct msg: "gateway proxy not found", } } else { - for _, addr := range gatewayProxy.Spec.StatusAddress { - if addr == "" { - continue - } + statusAddresses, err := r.resolveStatusAddresses(ctx, &gatewayProxy) + if err != nil { + // fail the reconcile so a missing or invalid publish Service retries + // with backoff, mirroring the Ingress status path + r.Log.Error(err, "failed to resolve gateway status addresses", "gateway", req.NamespacedName) + return ctrl.Result{}, err + } + for _, addr := range statusAddresses { addrType := gatewayv1.IPAddressType if net.ParseIP(addr) == nil { addrType = gatewayv1.HostnameAddressType @@ -259,6 +263,38 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct return ctrl.Result{}, nil } +// resolveStatusAddresses returns the addresses to publish in +// Gateway.status.addresses: the statically configured statusAddress if set, +// otherwise the external addresses of the Service named by publishService. +// This mirrors the Ingress status path, so the same GatewayProxy yields the +// same addresses for both APIs. +func (r *GatewayReconciler) resolveStatusAddresses( + ctx context.Context, + gatewayProxy *v1alpha1.GatewayProxy, +) ([]string, error) { + if len(gatewayProxy.Spec.StatusAddress) > 0 { + return utils.Filter(gatewayProxy.Spec.StatusAddress, func(addr string) bool { + return addr != "" + }), nil + } + + if gatewayProxy.Spec.PublishService == "" { + return nil, nil + } + + // a bare name is resolved against the GatewayProxy's namespace + svc, err := resolvePublishService(ctx, r.Client, gatewayProxy.Spec.PublishService, gatewayProxy.GetNamespace()) + if err != nil { + return nil, err + } + if svc.Spec.Type != corev1.ServiceTypeLoadBalancer { + r.Log.Info("publish service is not a LoadBalancer; no address to publish", + "service", gatewayProxy.Spec.PublishService, "type", svc.Spec.Type) + return nil, nil + } + return serviceLoadBalancerAddresses(svc), nil +} + func (r *GatewayReconciler) matchesGatewayClass(obj client.Object) bool { gateway, ok := obj.(*gatewayv1.GatewayClass) if !ok { diff --git a/internal/controller/ingress_controller.go b/internal/controller/ingress_controller.go index 517be554..6a849c71 100644 --- a/internal/controller/ingress_controller.go +++ b/internal/controller/ingress_controller.go @@ -750,35 +750,23 @@ func (r *IngressReconciler) updateStatus(ctx context.Context, tctx *provider.Tra // 2. if the IngressStatusAddress is not configured, try to use the PublishService publishService := gatewayProxy.Spec.PublishService if publishService != "" { - // parse the namespace/name format - namespace, name, err := SplitMetaNamespaceKey(publishService) - if err != nil { - return fmt.Errorf("invalid ingress-publish-service format: %s, expected format: namespace/name", publishService) - } // if the namespace is not specified, use the ingress namespace - if namespace == "" { - namespace = ingress.Namespace - } - - svc := &corev1.Service{} - if err := r.Get(ctx, client.ObjectKey{Namespace: namespace, Name: name}, svc); err != nil { - return fmt.Errorf("failed to get publish service %s: %w", publishService, err) + svc, err := resolvePublishService(ctx, r.Client, publishService, ingress.Namespace) + if err != nil { + return err } + namespace, name := svc.Namespace, svc.Name switch svc.Spec.Type { case corev1.ServiceTypeLoadBalancer: - // get the LoadBalancer IP and Hostname of the service - for _, ip := range svc.Status.LoadBalancer.Ingress { - if ip.IP != "" { - loadBalancerStatus.Ingress = append(loadBalancerStatus.Ingress, networkingv1.IngressLoadBalancerIngress{ - IP: ip.IP, - }) - } - if ip.Hostname != "" { - loadBalancerStatus.Ingress = append(loadBalancerStatus.Ingress, networkingv1.IngressLoadBalancerIngress{ - Hostname: ip.Hostname, - }) + for _, addr := range serviceLoadBalancerAddresses(svc) { + lbIngress := networkingv1.IngressLoadBalancerIngress{} + if net.ParseIP(addr) != nil { + lbIngress.IP = addr + } else { + lbIngress.Hostname = addr } + loadBalancerStatus.Ingress = append(loadBalancerStatus.Ingress, lbIngress) } case corev1.ServiceTypeClusterIP: // For ClusterIP services, propagate load balancer status from any other diff --git a/internal/controller/utils.go b/internal/controller/utils.go index 445da8ea..65e5f861 100644 --- a/internal/controller/utils.go +++ b/internal/controller/utils.go @@ -1238,22 +1238,6 @@ func validateListenerFrontendValidation( } } -// SplitMetaNamespaceKey returns the namespace and name that -// MetaNamespaceKeyFunc encoded into key. -func SplitMetaNamespaceKey(key string) (namespace, name string, err error) { - parts := strings.Split(key, "/") - switch len(parts) { - case 1: - // name only, no namespace - return "", parts[0], nil - case 2: - // namespace and name - return parts[0], parts[1], nil - } - - return "", "", fmt.Errorf("unexpected key format: %q", key) -} - func ProcessGatewayProxy(r client.Client, log logr.Logger, tctx *provider.TranslateContext, gateway *gatewayv1.Gateway, rk types.NamespacedNameKind) error { if gateway == nil { return nil @@ -2162,3 +2146,42 @@ func deduplicateGatewayStatusAddresses(addrs []gatewayv1.GatewayStatusAddress) [ return a.Value == b.Value }) } + +// resolvePublishService looks up the Service named by publishService, given as +// "namespace/name" or as a bare name resolved against defaultNamespace. Callers +// map the Service's addresses into whichever status shape their API uses. +func resolvePublishService( + ctx context.Context, + c client.Client, + publishService, defaultNamespace string, +) (*corev1.Service, error) { + namespace, name, err := utils.SplitMetaNamespaceKey(publishService) + if err != nil { + return nil, fmt.Errorf("invalid publish service format: %s, expected format: namespace/name", publishService) + } + // if the namespace is not specified, use the caller's namespace + if namespace == "" { + namespace = defaultNamespace + } + + svc := &corev1.Service{} + if err := c.Get(ctx, client.ObjectKey{Namespace: namespace, Name: name}, svc); err != nil { + return nil, fmt.Errorf("failed to get publish service %s: %w", publishService, err) + } + return svc, nil +} + +// serviceLoadBalancerAddresses flattens the Service's LoadBalancer ingress +// entries into address strings, keeping per-entry order: IP before hostname. +func serviceLoadBalancerAddresses(svc *corev1.Service) []string { + var addrs []string + for _, ing := range svc.Status.LoadBalancer.Ingress { + if ing.IP != "" { + addrs = append(addrs, ing.IP) + } + if ing.Hostname != "" { + addrs = append(addrs, ing.Hostname) + } + } + return addrs +} diff --git a/internal/controller/utils_publishservice_test.go b/internal/controller/utils_publishservice_test.go new file mode 100644 index 00000000..111da78c --- /dev/null +++ b/internal/controller/utils_publishservice_test.go @@ -0,0 +1,41 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package controller + +import ( + "testing" + + "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" +) + +func TestServiceLoadBalancerAddresses(t *testing.T) { + svc := &corev1.Service{ + Status: corev1.ServiceStatus{ + LoadBalancer: corev1.LoadBalancerStatus{ + Ingress: []corev1.LoadBalancerIngress{ + {IP: "10.0.0.1", Hostname: "a.example.com"}, + {Hostname: "b.example.com"}, + {}, + }, + }, + }, + } + assert.Equal(t, []string{"10.0.0.1", "a.example.com", "b.example.com"}, serviceLoadBalancerAddresses(svc)) + assert.Nil(t, serviceLoadBalancerAddresses(&corev1.Service{})) +} diff --git a/internal/utils/k8s.go b/internal/utils/k8s.go index 55ea03ef..f4aaa6a8 100644 --- a/internal/utils/k8s.go +++ b/internal/utils/k8s.go @@ -18,8 +18,10 @@ package utils import ( + "fmt" "net" "regexp" + "strings" networkingv1 "k8s.io/api/networking/v1" networkingv1beta1 "k8s.io/api/networking/v1beta1" @@ -123,3 +125,19 @@ func GetIngressClassV1beta1ParametersNamespace(ingressClass networkingv1beta1.In } return namespace } + +// SplitMetaNamespaceKey returns the namespace and name that +// MetaNamespaceKeyFunc encoded into key. +func SplitMetaNamespaceKey(key string) (namespace, name string, err error) { + parts := strings.Split(key, "/") + switch len(parts) { + case 1: + // name only, no namespace + return "", parts[0], nil + case 2: + // namespace and name + return parts[0], parts[1], nil + } + + return "", "", fmt.Errorf("unexpected key format: %q", key) +} diff --git a/test/e2e/gatewayapi/gateway.go b/test/e2e/gatewayapi/gateway.go index 8d660fd6..fd4690dd 100644 --- a/test/e2e/gatewayapi/gateway.go +++ b/test/e2e/gatewayapi/gateway.go @@ -26,6 +26,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" k8stypes "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" @@ -831,4 +832,182 @@ spec: assertGatewayAddress(gatewayName, updatedAddr, gatewayv1.HostnameAddressType) }) }) + + Context("Gateway Status Address from publishService", func() { + var gatewayProxyWithPublishServiceYaml = ` +apiVersion: apisix.apache.org/v1alpha1 +kind: GatewayProxy +metadata: + name: apisix-proxy-config + namespace: %s +spec: + publishService: %s/%s + provider: + type: ControlPlane + controlPlane: + endpoints: + - %s + auth: + type: AdminKey + adminKey: + value: "%s" +` + var defaultGatewayClass = ` +apiVersion: gateway.networking.k8s.io/v1 +kind: GatewayClass +metadata: + name: %s +spec: + controllerName: "%s" +` + var defaultGateway = ` +apiVersion: gateway.networking.k8s.io/v1 +kind: Gateway +metadata: + name: %s +spec: + gatewayClassName: %s + listeners: + - name: http + protocol: HTTP + port: 80 + infrastructure: + parametersRef: + group: apisix.apache.org + kind: GatewayProxy + name: apisix-proxy-config +` + const publishServiceName = "apisix-publish-svc" + + // createPublishService creates a LoadBalancer Service and, because kind has + // no cloud provider to do it, writes the external address into its status. + createPublishService := func(lbIngress ...corev1.LoadBalancerIngress) { + svcYaml := fmt.Sprintf(` +apiVersion: v1 +kind: Service +metadata: + name: %s + namespace: %s +spec: + type: LoadBalancer + selector: + app: httpbin + ports: + - port: 80 + targetPort: 80 +`, publishServiceName, s.Namespace()) + Expect(s.CreateResourceFromStringWithNamespace(svcYaml, s.Namespace())). + NotTo(HaveOccurred(), "creating publish Service") + setPublishServiceAddress(s, publishServiceName, lbIngress...) + } + + createGatewayClassAndGateway := func(gatewayClassName, gatewayName string) { + By("create GatewayClass") + Expect(s.CreateResourceFromStringWithNamespace( + fmt.Sprintf(defaultGatewayClass, gatewayClassName, s.GetControllerName()), ""), + ).NotTo(HaveOccurred(), "creating GatewayClass") + + By("create Gateway") + Expect(s.CreateResourceFromStringWithNamespace( + fmt.Sprintf(defaultGateway, gatewayName, gatewayClassName), s.Namespace()), + ).NotTo(HaveOccurred(), "creating Gateway") + } + + assertGatewayAddresses := func(gatewayName string, expected ...gatewayv1.GatewayStatusAddress) { + s.RetryAssertion(func() error { + var gateway gatewayv1.Gateway + if err := s.GetKubeClient().Get(context.Background(), k8stypes.NamespacedName{ + Name: gatewayName, + Namespace: s.Namespace(), + }, &gateway); err != nil { + return err + } + addrs := gateway.Status.Addresses + if len(addrs) != len(expected) { + return fmt.Errorf("expected %d status addresses, got %d: %+v", len(expected), len(addrs), addrs) + } + for i, want := range expected { + if addrs[i].Value != want.Value { + return fmt.Errorf("address %d: expected value %s, got %s", i, want.Value, addrs[i].Value) + } + if addrs[i].Type == nil { + return fmt.Errorf("address %d: expected type to be set, got nil", i) + } + if *addrs[i].Type != *want.Type { + return fmt.Errorf("address %d: expected type %s, got %s", i, *want.Type, *addrs[i].Type) + } + } + return nil + }).ShouldNot(HaveOccurred(), "check Gateway status addresses") + } + + It("falls back to publishService when statusAddress is empty", func() { + By("create LoadBalancer publish Service with an IP and a hostname") + createPublishService( + corev1.LoadBalancerIngress{IP: "10.99.88.77"}, + corev1.LoadBalancerIngress{Hostname: "lb.example.com"}, + ) + + By("create GatewayProxy with publishService and no statusAddress") + Expect(s.CreateResourceFromString(fmt.Sprintf(gatewayProxyWithPublishServiceYaml, + s.Namespace(), s.Namespace(), publishServiceName, s.Deployer.GetAdminEndpoint(), s.AdminKey()), + )).NotTo(HaveOccurred(), "creating GatewayProxy") + + createGatewayClassAndGateway(s.Namespace(), s.Namespace()) + + By("check Gateway status addresses come from the publish Service") + assertGatewayAddresses(s.Namespace(), + gatewayv1.GatewayStatusAddress{Type: ptr.To(gatewayv1.IPAddressType), Value: "10.99.88.77"}, + gatewayv1.GatewayStatusAddress{Type: ptr.To(gatewayv1.HostnameAddressType), Value: "lb.example.com"}, + ) + }) + + It("prefers statusAddress over publishService when both are set", func() { + By("create LoadBalancer publish Service") + createPublishService(corev1.LoadBalancerIngress{IP: "10.99.88.77"}) + + By("create GatewayProxy with both statusAddress and publishService") + Expect(s.CreateResourceFromString(fmt.Sprintf(` +apiVersion: apisix.apache.org/v1alpha1 +kind: GatewayProxy +metadata: + name: apisix-proxy-config + namespace: %s +spec: + statusAddress: + - 192.168.1.100 + publishService: %s/%s + provider: + type: ControlPlane + controlPlane: + endpoints: + - %s + auth: + type: AdminKey + adminKey: + value: "%s" +`, s.Namespace(), s.Namespace(), publishServiceName, s.Deployer.GetAdminEndpoint(), s.AdminKey()), + )).NotTo(HaveOccurred(), "creating GatewayProxy") + + createGatewayClassAndGateway(s.Namespace(), s.Namespace()) + + By("check only the static statusAddress is published") + assertGatewayAddresses(s.Namespace(), + gatewayv1.GatewayStatusAddress{Type: ptr.To(gatewayv1.IPAddressType), Value: "192.168.1.100"}) + }) + }) }) + +// setPublishServiceAddress writes lbIngress into the Service's +// status.loadBalancer.ingress, standing in for the cloud provider that assigns +// a LoadBalancer address in a real cluster. +func setPublishServiceAddress(s *scaffold.Scaffold, name string, lbIngress ...corev1.LoadBalancerIngress) { + var svc corev1.Service + Expect(s.GetKubeClient().Get(context.Background(), k8stypes.NamespacedName{ + Name: name, + Namespace: s.Namespace(), + }, &svc)).NotTo(HaveOccurred(), "getting publish Service") + svc.Status.LoadBalancer.Ingress = lbIngress + Expect(s.GetKubeClient().Status().Update(context.Background(), &svc)). + NotTo(HaveOccurred(), "updating publish Service status") +} From cd0642df29963fe1f1c1e82373f5c590ca5911af Mon Sep 17 00:00:00 2001 From: janiussyafiq Date: Fri, 14 Aug 2026 12:24:31 +0800 Subject: [PATCH 3/5] fix: do not let an unresolvable publish Service block the data plane push An error from resolveStatusAddresses returned before Provider.Update, so a publishService typo or a not-yet-created Service stopped the Gateway from being pushed to APISIX and skipped the status write entirely. Carry the error to the end of the reconcile instead: the data plane push and the condition updates still happen, previously published addresses are kept, and the returned error preserves the backoff retry. --- internal/controller/gateway_controller.go | 15 +- .../gateway_controller_publishservice_test.go | 133 ++++++++++++++++++ 2 files changed, 141 insertions(+), 7 deletions(-) create mode 100644 internal/controller/gateway_controller_publishservice_test.go diff --git a/internal/controller/gateway_controller.go b/internal/controller/gateway_controller.go index aaf599c9..c0193684 100644 --- a/internal/controller/gateway_controller.go +++ b/internal/controller/gateway_controller.go @@ -183,7 +183,10 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct } } - var addrs []gatewayv1.GatewayStatusAddress + var ( + addrs []gatewayv1.GatewayStatusAddress + addrResolveErr error + ) rk := utils.NamespacedNameKind(gateway) @@ -196,10 +199,8 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct } else { statusAddresses, err := r.resolveStatusAddresses(ctx, &gatewayProxy) if err != nil { - // fail the reconcile so a missing or invalid publish Service retries - // with backoff, mirroring the Ingress status path r.Log.Error(err, "failed to resolve gateway status addresses", "gateway", req.NamespacedName) - return ctrl.Result{}, err + addrResolveErr = err } for _, addr := range statusAddresses { addrType := gatewayv1.IPAddressType @@ -233,7 +234,7 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct accepted := SetGatewayConditionAccepted(gateway, acceptStatus.status, acceptStatus.msg) programmed := SetGatewayConditionProgrammed(gateway, conditionProgrammedStatus, conditionProgrammedMsg) - addressesChanged := !reflect.DeepEqual(gateway.Status.Addresses, addrs) + addressesChanged := addrResolveErr == nil && !reflect.DeepEqual(gateway.Status.Addresses, addrs) if accepted || programmed || addressesChanged || len(listenerStatuses) > 0 { if addressesChanged { gateway.Status.Addresses = addrs @@ -257,10 +258,10 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct }), }) - return ctrl.Result{}, nil + return ctrl.Result{}, addrResolveErr } - return ctrl.Result{}, nil + return ctrl.Result{}, addrResolveErr } // resolveStatusAddresses returns the addresses to publish in diff --git a/internal/controller/gateway_controller_publishservice_test.go b/internal/controller/gateway_controller_publishservice_test.go new file mode 100644 index 00000000..d9ed652f --- /dev/null +++ b/internal/controller/gateway_controller_publishservice_test.go @@ -0,0 +1,133 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package controller + +import ( + "context" + "net/http" + "testing" + + "github.com/go-logr/logr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + k8stypes "k8s.io/apimachinery/pkg/types" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + + "github.com/apache/apisix-ingress-controller/api/v1alpha1" + "github.com/apache/apisix-ingress-controller/internal/controller/config" + "github.com/apache/apisix-ingress-controller/internal/controller/status" + "github.com/apache/apisix-ingress-controller/internal/provider" +) + +// recordingProvider counts data plane pushes so tests can assert whether a +// reconcile reached Provider.Update. +type recordingProvider struct { + updated int +} + +func (p *recordingProvider) Update(context.Context, *provider.TranslateContext, client.Object) error { + p.updated++ + return nil +} +func (p *recordingProvider) Delete(context.Context, client.Object) error { return nil } +func (p *recordingProvider) Start(context.Context) error { return nil } +func (p *recordingProvider) NeedLeaderElection() bool { return false } +func (p *recordingProvider) Register(string, *http.ServeMux) {} + +type recordingUpdater struct { + updates []status.Update +} + +func (u *recordingUpdater) Update(update status.Update) { u.updates = append(u.updates, update) } + +// A publishService that cannot be resolved (typo, or the Service not created +// yet) is a status-only problem: the reconcile must still push the Gateway to +// the data plane and keep any previously published addresses, returning the +// error only so the resolution retries with backoff. +func TestGatewayReconcilePublishServiceResolveErrorDoesNotBlockDataPlane(t *testing.T) { + scheme := runtime.NewScheme() + require.NoError(t, clientgoscheme.AddToScheme(scheme)) + require.NoError(t, gatewayv1.Install(scheme)) + require.NoError(t, v1alpha1.AddToScheme(scheme)) + + gatewayClass := &gatewayv1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{Name: "apisix"}, + Spec: gatewayv1.GatewayClassSpec{ + ControllerName: gatewayv1.GatewayController(config.ControllerConfig.ControllerName), + }, + } + gatewayProxy := &v1alpha1.GatewayProxy{ + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "proxy"}, + Spec: v1alpha1.GatewayProxySpec{ + PublishService: "missing-svc", + }, + } + addrType := gatewayv1.IPAddressType + previousAddrs := []gatewayv1.GatewayStatusAddress{{Type: &addrType, Value: "203.0.113.10"}} + gateway := &gatewayv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "gw"}, + Spec: gatewayv1.GatewaySpec{ + GatewayClassName: "apisix", + Infrastructure: &gatewayv1.GatewayInfrastructure{ + ParametersRef: &gatewayv1.LocalParametersReference{ + Group: gatewayv1.Group(v1alpha1.GroupVersion.Group), + Kind: KindGatewayProxy, + Name: "proxy", + }, + }, + }, + Status: gatewayv1.GatewayStatus{Addresses: previousAddrs}, + } + + cli := fake.NewClientBuilder().WithScheme(scheme). + WithObjects(gatewayClass, gatewayProxy, gateway). + WithStatusSubresource(gateway). + Build() + + prov := &recordingProvider{} + updater := &recordingUpdater{} + r := &GatewayReconciler{ + Client: cli, + Scheme: scheme, + Log: logr.Discard(), + Provider: prov, + Updater: updater, + } + + result, err := r.Reconcile(context.Background(), + ctrl.Request{NamespacedName: k8stypes.NamespacedName{Namespace: "default", Name: "gw"}}) + + assert.Error(t, err, "the resolve failure must be returned so it retries with backoff") + assert.Equal(t, ctrl.Result{}, result) + assert.Equal(t, 1, prov.updated, "Provider.Update must run even when the publish Service cannot be resolved") + + require.Len(t, updater.updates, 1, "conditions must still be written") + mutated, ok := updater.updates[0].Mutator.Mutate(gateway).(*gatewayv1.Gateway) + require.True(t, ok) + assert.Equal(t, previousAddrs, mutated.Status.Addresses, + "previously published addresses must survive a transient resolve failure") + assert.True(t, meta.IsStatusConditionTrue(mutated.Status.Conditions, string(gatewayv1.GatewayConditionAccepted)), + "an unresolvable publish Service must not flip Accepted to False") +} From 2489162b30df6a979ea4580e4aa936f5ead819ca Mon Sep 17 00:00:00 2001 From: janiussyafiq Date: Fri, 14 Aug 2026 12:40:05 +0800 Subject: [PATCH 4/5] fix: resolve bare Ingress publishService names against the GatewayProxy namespace The Ingress status path defaulted a bare publishService name to the namespace of the Ingress being reconciled, while the Gateway path uses the GatewayProxy's namespace. The publish Service lives next to the GatewayProxy, so the Ingress rule only worked when the Ingress happened to share that namespace. Unify on the GatewayProxy's namespace and document the rule. --- docs/en/latest/reference/example.md | 3 +- internal/controller/gateway_controller.go | 3 +- internal/controller/ingress_controller.go | 5 +- .../ingress_controller_publishservice_test.go | 89 +++++++++++++++++++ 4 files changed, 95 insertions(+), 5 deletions(-) create mode 100644 internal/controller/ingress_controller_publishservice_test.go diff --git a/docs/en/latest/reference/example.md b/docs/en/latest/reference/example.md index 72b356e9..06399946 100644 --- a/docs/en/latest/reference/example.md +++ b/docs/en/latest/reference/example.md @@ -1309,7 +1309,8 @@ spec: publishService: apisix-ee-3-gateway-gateway ``` -When using `publishService`, the controller will use the endpoint of this Service to update the status information of the Ingress resource. The format can be either `namespace/svc-name` or simply `svc-name` if the default namespace is correctly set. +When using `publishService`, the controller will use the endpoint of this Service to update the status information of the Ingress resource. +The format can be either `namespace/svc-name` or simply `svc-name`, in which case the name resolves against the namespace of the GatewayProxy. - If the Service is of `LoadBalancer` type, the controller uses its external IP or hostname. - If the Service is of `ClusterIP` type, the controller propagates the hostname from any Ingress resources that reference that Service. diff --git a/internal/controller/gateway_controller.go b/internal/controller/gateway_controller.go index c0193684..b1a298bb 100644 --- a/internal/controller/gateway_controller.go +++ b/internal/controller/gateway_controller.go @@ -267,8 +267,7 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct // resolveStatusAddresses returns the addresses to publish in // Gateway.status.addresses: the statically configured statusAddress if set, // otherwise the external addresses of the Service named by publishService. -// This mirrors the Ingress status path, so the same GatewayProxy yields the -// same addresses for both APIs. +// A bare Service name resolves against the GatewayProxy's namespace. func (r *GatewayReconciler) resolveStatusAddresses( ctx context.Context, gatewayProxy *v1alpha1.GatewayProxy, diff --git a/internal/controller/ingress_controller.go b/internal/controller/ingress_controller.go index 6a849c71..b218794a 100644 --- a/internal/controller/ingress_controller.go +++ b/internal/controller/ingress_controller.go @@ -750,8 +750,9 @@ func (r *IngressReconciler) updateStatus(ctx context.Context, tctx *provider.Tra // 2. if the IngressStatusAddress is not configured, try to use the PublishService publishService := gatewayProxy.Spec.PublishService if publishService != "" { - // if the namespace is not specified, use the ingress namespace - svc, err := resolvePublishService(ctx, r.Client, publishService, ingress.Namespace) + // a bare name resolves against the GatewayProxy's namespace, where + // the publish Service lives, matching the Gateway API path + svc, err := resolvePublishService(ctx, r.Client, publishService, gatewayProxy.GetNamespace()) if err != nil { return err } diff --git a/internal/controller/ingress_controller_publishservice_test.go b/internal/controller/ingress_controller_publishservice_test.go new file mode 100644 index 00000000..0794fdbf --- /dev/null +++ b/internal/controller/ingress_controller_publishservice_test.go @@ -0,0 +1,89 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package controller + +import ( + "context" + "testing" + + "github.com/go-logr/logr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "github.com/apache/apisix-ingress-controller/api/v1alpha1" + "github.com/apache/apisix-ingress-controller/internal/provider" + "github.com/apache/apisix-ingress-controller/internal/utils" +) + +// A bare publishService name must resolve against the GatewayProxy's +// namespace, matching the Gateway API path: the publish Service lives next to +// the GatewayProxy, not in the namespace of whichever Ingress is being +// reconciled. +func TestIngressUpdateStatusBarePublishServiceUsesGatewayProxyNamespace(t *testing.T) { + scheme := runtime.NewScheme() + require.NoError(t, clientgoscheme.AddToScheme(scheme)) + require.NoError(t, v1alpha1.AddToScheme(scheme)) + + gatewayProxy := v1alpha1.GatewayProxy{ + ObjectMeta: metav1.ObjectMeta{Namespace: "infra", Name: "proxy"}, + Spec: v1alpha1.GatewayProxySpec{ + PublishService: "apisix-lb", + }, + } + publishSvc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Namespace: "infra", Name: "apisix-lb"}, + Spec: corev1.ServiceSpec{Type: corev1.ServiceTypeLoadBalancer}, + Status: corev1.ServiceStatus{ + LoadBalancer: corev1.LoadBalancerStatus{ + Ingress: []corev1.LoadBalancerIngress{{IP: "203.0.113.20"}}, + }, + }, + } + ingressClass := &networkingv1.IngressClass{ + ObjectMeta: metav1.ObjectMeta{Name: "apisix"}, + } + ingress := &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{Namespace: "app", Name: "ing"}, + } + + cli := fake.NewClientBuilder().WithScheme(scheme). + WithObjects(publishSvc, ingressClass, ingress). + Build() + + updater := &recordingUpdater{} + r := &IngressReconciler{Client: cli, Log: logr.Discard(), Updater: updater} + + tctx := provider.NewDefaultTranslateContext(context.Background()) + tctx.GatewayProxies[utils.NamespacedNameKind(ingressClass)] = gatewayProxy + + err := r.updateStatus(context.Background(), tctx, ingress, ingressClass) + require.NoError(t, err, + "a bare name must resolve in the GatewayProxy's namespace, not the Ingress's") + + require.Len(t, updater.updates, 1) + mutated, ok := updater.updates[0].Mutator.Mutate(ingress).(*networkingv1.Ingress) + require.True(t, ok) + assert.Equal(t, []networkingv1.IngressLoadBalancerIngress{{IP: "203.0.113.20"}}, + mutated.Status.LoadBalancer.Ingress) +} From 7a430bb705408b65e1f5e357457f4f149917df0c Mon Sep 17 00:00:00 2001 From: janiussyafiq Date: Fri, 14 Aug 2026 17:00:34 +0800 Subject: [PATCH 5/5] fix: report an unresolvable publish Service on the Programmed condition Returning the resolve error made a publishService typo retry forever and count toward controller_runtime_reconcile_errors_total, dressing a user configuration problem as a controller failure. Classify it instead: resolvePublishService returns a ReasonError with AddressNotAssigned for a bad format or a missing Service, and the Gateway reconcile turns that into Programmed=False with the message plus a quiet one-minute requeue (needed until Service events are watched). Other lookup failures keep the error return and backoff. --- internal/controller/gateway_controller.go | 36 +++-- .../gateway_controller_publishservice_test.go | 125 +++++++++++++++--- internal/controller/utils.go | 20 ++- 3 files changed, 147 insertions(+), 34 deletions(-) diff --git a/internal/controller/gateway_controller.go b/internal/controller/gateway_controller.go index b1a298bb..72d23eee 100644 --- a/internal/controller/gateway_controller.go +++ b/internal/controller/gateway_controller.go @@ -23,6 +23,7 @@ import ( "fmt" "net" "reflect" + "time" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" @@ -46,6 +47,10 @@ import ( pkgutils "github.com/apache/apisix-ingress-controller/pkg/utils" ) +// publishServiceRetryInterval polls an unresolvable publishService, since +// Service events are not watched. +const publishServiceRetryInterval = time.Minute + // GatewayReconciler reconciles a Gateway object. type GatewayReconciler struct { //nolint:revive client.Client @@ -161,6 +166,7 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct } conditionProgrammedStatus, conditionProgrammedMsg := true, "Programmed" + conditionProgrammedReason := string(gatewayv1.GatewayReasonProgrammed) r.Log.Info("gateway has been accepted", "gateway", gateway.GetName()) type conditionStatus struct { @@ -184,8 +190,10 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct } var ( - addrs []gatewayv1.GatewayStatusAddress - addrResolveErr error + addrs []gatewayv1.GatewayStatusAddress + addrResolveFailed bool + addrResolveErr error + addrRetryAfter time.Duration ) rk := utils.NamespacedNameKind(gateway) @@ -199,8 +207,20 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct } else { statusAddresses, err := r.resolveStatusAddresses(ctx, &gatewayProxy) if err != nil { - r.Log.Error(err, "failed to resolve gateway status addresses", "gateway", req.NamespacedName) - addrResolveErr = err + addrResolveFailed = true + if internaltypes.IsSomeReasonError(err, gatewayv1.GatewayReasonAddressNotAssigned) { + // a config problem, not a controller failure: report it on the + // Programmed condition instead of the reconcile error metric + r.Log.Info("cannot resolve gateway status addresses", + "gateway", req.NamespacedName, "reason", err.Error()) + conditionProgrammedStatus = false + conditionProgrammedMsg = err.Error() + conditionProgrammedReason = string(gatewayv1.GatewayReasonAddressNotAssigned) + addrRetryAfter = publishServiceRetryInterval + } else { + r.Log.Error(err, "failed to resolve gateway status addresses", "gateway", req.NamespacedName) + addrResolveErr = err + } } for _, addr := range statusAddresses { addrType := gatewayv1.IPAddressType @@ -233,8 +253,8 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct } accepted := SetGatewayConditionAccepted(gateway, acceptStatus.status, acceptStatus.msg) - programmed := SetGatewayConditionProgrammed(gateway, conditionProgrammedStatus, conditionProgrammedMsg) - addressesChanged := addrResolveErr == nil && !reflect.DeepEqual(gateway.Status.Addresses, addrs) + programmed := SetGatewayConditionProgrammed(gateway, conditionProgrammedStatus, conditionProgrammedReason, conditionProgrammedMsg) + addressesChanged := !addrResolveFailed && !reflect.DeepEqual(gateway.Status.Addresses, addrs) if accepted || programmed || addressesChanged || len(listenerStatuses) > 0 { if addressesChanged { gateway.Status.Addresses = addrs @@ -258,10 +278,10 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct }), }) - return ctrl.Result{}, addrResolveErr + return ctrl.Result{RequeueAfter: addrRetryAfter}, addrResolveErr } - return ctrl.Result{}, addrResolveErr + return ctrl.Result{RequeueAfter: addrRetryAfter}, addrResolveErr } // resolveStatusAddresses returns the addresses to publish in diff --git a/internal/controller/gateway_controller_publishservice_test.go b/internal/controller/gateway_controller_publishservice_test.go index d9ed652f..116eacf5 100644 --- a/internal/controller/gateway_controller_publishservice_test.go +++ b/internal/controller/gateway_controller_publishservice_test.go @@ -25,6 +25,8 @@ import ( "github.com/go-logr/logr" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -33,6 +35,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" "github.com/apache/apisix-ingress-controller/api/v1alpha1" @@ -62,11 +65,21 @@ type recordingUpdater struct { func (u *recordingUpdater) Update(update status.Update) { u.updates = append(u.updates, update) } -// A publishService that cannot be resolved (typo, or the Service not created -// yet) is a status-only problem: the reconcile must still push the Gateway to -// the data plane and keep any previously published addresses, returning the -// error only so the resolution retries with backoff. -func TestGatewayReconcilePublishServiceResolveErrorDoesNotBlockDataPlane(t *testing.T) { +var gatewayPreviousAddrs = func() []gatewayv1.GatewayStatusAddress { + addrType := gatewayv1.IPAddressType + return []gatewayv1.GatewayStatusAddress{{Type: &addrType, Value: "203.0.113.10"}} +}() + +// newGatewayPublishServiceFixture builds a Gateway with previously published +// addresses, wired to a GatewayProxy with the given publishService. +func newGatewayPublishServiceFixture( + t *testing.T, + publishService string, + interceptorFuncs interceptor.Funcs, + extraObjects ...client.Object, +) (*GatewayReconciler, *recordingProvider, *recordingUpdater) { + t.Helper() + scheme := runtime.NewScheme() require.NoError(t, clientgoscheme.AddToScheme(scheme)) require.NoError(t, gatewayv1.Install(scheme)) @@ -81,11 +94,9 @@ func TestGatewayReconcilePublishServiceResolveErrorDoesNotBlockDataPlane(t *test gatewayProxy := &v1alpha1.GatewayProxy{ ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "proxy"}, Spec: v1alpha1.GatewayProxySpec{ - PublishService: "missing-svc", + PublishService: publishService, }, } - addrType := gatewayv1.IPAddressType - previousAddrs := []gatewayv1.GatewayStatusAddress{{Type: &addrType, Value: "203.0.113.10"}} gateway := &gatewayv1.Gateway{ ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "gw"}, Spec: gatewayv1.GatewaySpec{ @@ -98,36 +109,108 @@ func TestGatewayReconcilePublishServiceResolveErrorDoesNotBlockDataPlane(t *test }, }, }, - Status: gatewayv1.GatewayStatus{Addresses: previousAddrs}, + Status: gatewayv1.GatewayStatus{Addresses: gatewayPreviousAddrs}, } + objects := append([]client.Object{gatewayClass, gatewayProxy, gateway}, extraObjects...) cli := fake.NewClientBuilder().WithScheme(scheme). - WithObjects(gatewayClass, gatewayProxy, gateway). + WithObjects(objects...). WithStatusSubresource(gateway). + WithInterceptorFuncs(interceptorFuncs). Build() prov := &recordingProvider{} updater := &recordingUpdater{} - r := &GatewayReconciler{ + return &GatewayReconciler{ Client: cli, Scheme: scheme, Log: logr.Discard(), Provider: prov, Updater: updater, - } + }, prov, updater +} - result, err := r.Reconcile(context.Background(), +func reconcileGateway(t *testing.T, r *GatewayReconciler) (ctrl.Result, error) { + t.Helper() + return r.Reconcile(context.Background(), ctrl.Request{NamespacedName: k8stypes.NamespacedName{Namespace: "default", Name: "gw"}}) +} - assert.Error(t, err, "the resolve failure must be returned so it retries with backoff") - assert.Equal(t, ctrl.Result{}, result) - assert.Equal(t, 1, prov.updated, "Provider.Update must run even when the publish Service cannot be resolved") - +// mutatedGatewayStatus applies the single recorded status update and returns +// the Gateway status it would have written. +func mutatedGatewayStatus(t *testing.T, updater *recordingUpdater) gatewayv1.GatewayStatus { + t.Helper() require.Len(t, updater.updates, 1, "conditions must still be written") - mutated, ok := updater.updates[0].Mutator.Mutate(gateway).(*gatewayv1.Gateway) + mutated, ok := updater.updates[0].Mutator.Mutate(&gatewayv1.Gateway{}).(*gatewayv1.Gateway) require.True(t, ok) - assert.Equal(t, previousAddrs, mutated.Status.Addresses, - "previously published addresses must survive a transient resolve failure") - assert.True(t, meta.IsStatusConditionTrue(mutated.Status.Conditions, string(gatewayv1.GatewayConditionAccepted)), + return mutated.Status +} + +// A missing publish Service must not block the data plane push or count as a +// reconcile error: it surfaces as Programmed=False/AddressNotAssigned and a +// requeue picks the addresses up once the Service exists. +func TestGatewayReconcilePublishServiceNotFound(t *testing.T) { + r, prov, updater := newGatewayPublishServiceFixture(t, "missing-svc", interceptor.Funcs{}) + + result, err := reconcileGateway(t, r) + + assert.NoError(t, err, "a missing publish Service must not count as a reconcile error") + assert.Equal(t, publishServiceRetryInterval, result.RequeueAfter, + "must poll for the Service until a Service watch makes this event-driven") + assert.Equal(t, 1, prov.updated, "Provider.Update must run even when the publish Service cannot be resolved") + + gotStatus := mutatedGatewayStatus(t, updater) + programmed := meta.FindStatusCondition(gotStatus.Conditions, string(gatewayv1.GatewayConditionProgrammed)) + require.NotNil(t, programmed) + assert.Equal(t, metav1.ConditionFalse, programmed.Status) + assert.Equal(t, string(gatewayv1.GatewayReasonAddressNotAssigned), programmed.Reason) + assert.Contains(t, programmed.Message, "missing-svc") + assert.True(t, meta.IsStatusConditionTrue(gotStatus.Conditions, string(gatewayv1.GatewayConditionAccepted)), "an unresolvable publish Service must not flip Accepted to False") + assert.Equal(t, gatewayPreviousAddrs, gotStatus.Addresses, + "previously published addresses must survive a resolve failure") +} + +// An invalid publishService format is handled like NotFound: surfaced on the +// Programmed condition, not the reconcile error. +func TestGatewayReconcilePublishServiceBadFormat(t *testing.T) { + r, prov, updater := newGatewayPublishServiceFixture(t, "a/b/c", interceptor.Funcs{}) + + result, err := reconcileGateway(t, r) + + assert.NoError(t, err, "an invalid publishService value must not count as a reconcile error") + assert.Equal(t, publishServiceRetryInterval, result.RequeueAfter) + assert.Equal(t, 1, prov.updated) + + gotStatus := mutatedGatewayStatus(t, updater) + programmed := meta.FindStatusCondition(gotStatus.Conditions, string(gatewayv1.GatewayConditionProgrammed)) + require.NotNil(t, programmed) + assert.Equal(t, metav1.ConditionFalse, programmed.Status) + assert.Equal(t, string(gatewayv1.GatewayReasonAddressNotAssigned), programmed.Reason) + assert.Contains(t, programmed.Message, "a/b/c") +} + +// A non-NotFound lookup failure is a genuine API failure: returned as a +// reconcile error, without blaming the user's config on Programmed. +func TestGatewayReconcilePublishServiceAPIFailure(t *testing.T) { + apiDown := apierrors.NewInternalError(context.DeadlineExceeded) + r, prov, updater := newGatewayPublishServiceFixture(t, "some-svc", interceptor.Funcs{ + Get: func(ctx context.Context, cli client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + if _, isService := obj.(*corev1.Service); isService { + return apiDown + } + return cli.Get(ctx, key, obj, opts...) + }, + }) + + result, err := reconcileGateway(t, r) + + assert.ErrorIs(t, err, apiDown, "an API failure must be returned for backoff retry") + assert.Equal(t, ctrl.Result{}, result) + assert.Equal(t, 1, prov.updated, "Provider.Update must run even when the address lookup fails") + + gotStatus := mutatedGatewayStatus(t, updater) + assert.True(t, meta.IsStatusConditionTrue(gotStatus.Conditions, string(gatewayv1.GatewayConditionProgrammed)), + "an API failure is not a user configuration problem") + assert.Equal(t, gatewayPreviousAddrs, gotStatus.Addresses) } diff --git a/internal/controller/utils.go b/internal/controller/utils.go index 65e5f861..ce1b1bc1 100644 --- a/internal/controller/utils.go +++ b/internal/controller/utils.go @@ -241,11 +241,11 @@ func SetGatewayListenerConditionResolvedRefs(gw *gatewayv1.Gateway, listenerName return } -func SetGatewayConditionProgrammed(gw *gatewayv1.Gateway, status bool, message string) (ok bool) { +func SetGatewayConditionProgrammed(gw *gatewayv1.Gateway, status bool, reason, message string) (ok bool) { condition := metav1.Condition{ Type: string(gatewayv1.GatewayConditionProgrammed), Status: ConditionStatus(status), - Reason: string(gatewayv1.GatewayReasonProgrammed), + Reason: reason, ObservedGeneration: gw.GetGeneration(), Message: message, LastTransitionTime: metav1.Now(), @@ -2148,8 +2148,9 @@ func deduplicateGatewayStatusAddresses(addrs []gatewayv1.GatewayStatusAddress) [ } // resolvePublishService looks up the Service named by publishService, given as -// "namespace/name" or as a bare name resolved against defaultNamespace. Callers -// map the Service's addresses into whichever status shape their API uses. +// "namespace/name" or as a bare name resolved against defaultNamespace. +// A value that cannot work (bad format, no such Service) comes back as a +// ReasonError with GatewayReasonAddressNotAssigned. func resolvePublishService( ctx context.Context, c client.Client, @@ -2157,7 +2158,10 @@ func resolvePublishService( ) (*corev1.Service, error) { namespace, name, err := utils.SplitMetaNamespaceKey(publishService) if err != nil { - return nil, fmt.Errorf("invalid publish service format: %s, expected format: namespace/name", publishService) + return nil, types.ReasonError{ + Reason: string(gatewayv1.GatewayReasonAddressNotAssigned), + Message: fmt.Sprintf("invalid publish service format: %s, expected format: namespace/name", publishService), + } } // if the namespace is not specified, use the caller's namespace if namespace == "" { @@ -2166,6 +2170,12 @@ func resolvePublishService( svc := &corev1.Service{} if err := c.Get(ctx, client.ObjectKey{Namespace: namespace, Name: name}, svc); err != nil { + if k8serrors.IsNotFound(err) { + return nil, types.ReasonError{ + Reason: string(gatewayv1.GatewayReasonAddressNotAssigned), + Message: fmt.Sprintf("publish service %s/%s not found", namespace, name), + } + } return nil, fmt.Errorf("failed to get publish service %s: %w", publishService, err) } return svc, nil