diff --git a/cmd/gateway/commands.go b/cmd/gateway/commands.go index 20d6f639bd..43c9974e1a 100644 --- a/cmd/gateway/commands.go +++ b/cmd/gateway/commands.go @@ -69,6 +69,7 @@ func createStaticModeCommand() *cobra.Command { const ( gatewayFlag = "gateway" configFlag = "config" + serviceFlag = "service" updateGCStatusFlag = "update-gatewayclass-status" metricsDisableFlag = "metrics-disable" metricsSecureFlag = "metrics-secure-serving" @@ -86,6 +87,9 @@ func createStaticModeCommand() *cobra.Command { configName = stringValidatingValue{ validator: validateResourceName, } + serviceName = stringValidatingValue{ + validator: validateResourceName, + } disableMetrics bool metricsSecure bool metricsListenPort = intValidatingValue{ @@ -150,10 +154,13 @@ func createStaticModeCommand() *cobra.Command { Logger: logger, AtomicLevel: atom, GatewayClassName: gatewayClassName.value, - PodIP: podIP, - Namespace: namespace, GatewayNsName: gwNsName, UpdateGatewayClassStatus: updateGCStatus, + GatewayPodConfig: config.GatewayPodConfig{ + PodIP: podIP, + ServiceName: serviceName.value, + Namespace: namespace, + }, HealthConfig: config.HealthConfig{ Enabled: !disableHealth, Port: healthListenPort.value, @@ -196,6 +203,13 @@ func createStaticModeCommand() *cobra.Command { ` Lives in the same Namespace as the controller.`, ) + cmd.Flags().Var( + &serviceName, + serviceFlag, + `The name of the Service that fronts this NGINX Gateway Fabric Pod.`+ + ` Lives in the same Namespace as the controller.`, + ) + cmd.Flags().BoolVar( &updateGCStatus, updateGCStatusFlag, diff --git a/cmd/gateway/commands_test.go b/cmd/gateway/commands_test.go index 5f545520ce..1b8302def4 100644 --- a/cmd/gateway/commands_test.go +++ b/cmd/gateway/commands_test.go @@ -117,6 +117,7 @@ func TestStaticModeCmdFlagValidation(t *testing.T) { args: []string{ "--gateway=nginx-gateway/nginx", "--config=nginx-gateway-config", + "--service=nginx-gateway", "--update-gatewayclass-status=true", "--metrics-port=9114", "--metrics-disable", @@ -166,6 +167,22 @@ func TestStaticModeCmdFlagValidation(t *testing.T) { wantErr: true, expectedErrPrefix: `invalid argument "!@#$" for "-c, --config" flag: invalid format`, }, + { + name: "service is set to empty string", + args: []string{ + "--service=", + }, + wantErr: true, + expectedErrPrefix: `invalid argument "" for "--service" flag: must be set`, + }, + { + name: "service is set to invalid string", + args: []string{ + "--service=!@#$", + }, + wantErr: true, + expectedErrPrefix: `invalid argument "!@#$" for "--service" flag: invalid format`, + }, { name: "update-gatewayclass-status is set to empty string", args: []string{ diff --git a/conformance/Makefile b/conformance/Makefile index 9894ee2dae..4cea342f5f 100644 --- a/conformance/Makefile +++ b/conformance/Makefile @@ -10,7 +10,6 @@ TAG = latest PREFIX = conformance-test-runner NGF_MANIFEST=../deploy/manifests/nginx-gateway.yaml CRDS=../deploy/manifests/crds/ -SERVICE_MANIFEST=../deploy/manifests/service/nodeport.yaml STATIC_MANIFEST=provisioner/static-deployment.yaml PROVISIONER_MANIFEST=provisioner/provisioner.yaml NGINX_IMAGE=$(shell yq '.spec.template.spec.containers[1].image as $$nginx_ver | $$nginx_ver' $(STATIC_MANIFEST)) @@ -52,7 +51,6 @@ prepare-ngf-dependencies: update-ngf-manifest ## Install NGF dependencies on con kubectl wait --for=condition=available --timeout=60s deployment gateway-api-admission-server -n gateway-system kubectl apply -f $(CRDS) kubectl apply -f $(NGF_MANIFEST) - kubectl apply -f $(SERVICE_MANIFEST) .PHONY: deploy-updated-provisioner deploy-updated-provisioner: ## Update provisioner manifest and deploy to the configured kind cluster diff --git a/conformance/provisioner/static-deployment.yaml b/conformance/provisioner/static-deployment.yaml index 0895770db8..8d2b4c68a6 100644 --- a/conformance/provisioner/static-deployment.yaml +++ b/conformance/provisioner/static-deployment.yaml @@ -27,6 +27,7 @@ spec: - --gateway-ctlr-name=gateway.nginx.org/nginx-gateway-controller - --gatewayclass=nginx - --config=nginx-gateway-config + - --service=nginx-gateway - --metrics-disable - --health-port=8081 - --leader-election-lock-name=nginx-gateway-leader-election diff --git a/deploy/helm-chart/templates/deployment.yaml b/deploy/helm-chart/templates/deployment.yaml index d1561cacfa..bc7195b478 100644 --- a/deploy/helm-chart/templates/deployment.yaml +++ b/deploy/helm-chart/templates/deployment.yaml @@ -30,6 +30,7 @@ spec: - --gateway-ctlr-name={{ .Values.nginxGateway.gatewayControllerName }} - --gatewayclass={{ .Values.nginxGateway.gatewayClassName }} - --config={{ include "nginx-gateway.config-name" . }} + - --service={{ include "nginx-gateway.fullname" . }} {{- if .Values.metrics.enable }} - --metrics-port={{ .Values.metrics.port }} {{- if .Values.metrics.secure }} diff --git a/deploy/manifests/nginx-gateway.yaml b/deploy/manifests/nginx-gateway.yaml index 648785e01d..28c26b86b2 100644 --- a/deploy/manifests/nginx-gateway.yaml +++ b/deploy/manifests/nginx-gateway.yaml @@ -139,6 +139,7 @@ spec: - --gateway-ctlr-name=gateway.nginx.org/nginx-gateway-controller - --gatewayclass=nginx - --config=nginx-gateway-config + - --service=nginx-gateway - --metrics-port=9113 - --health-port=8081 - --leader-election-lock-name=nginx-gateway-leader-election diff --git a/docs/cli-help.md b/docs/cli-help.md index 48869c44c0..7c20ae892e 100644 --- a/docs/cli-help.md +++ b/docs/cli-help.md @@ -20,6 +20,7 @@ Flags: | `gatewayclass` | `string` | The name of the GatewayClass resource. Every NGINX Gateway Fabric must have a unique corresponding GatewayClass resource. | | `gateway` | `string` | The namespaced name of the Gateway resource to use. Must be of the form: `NAMESPACE/NAME`. If not specified, the control plane will process all Gateways for the configured GatewayClass. However, among them, it will choose the oldest resource by creation timestamp. If the timestamps are equal, it will choose the resource that appears first in alphabetical order by {namespace}/{name}. | | `config` | `string` | The name of the NginxGateway resource to be used for this controller's dynamic configuration. Lives in the same Namespace as the controller. | +| `service` | `string` | The name of the Service that fronts this NGINX Gateway Fabric Pod. Lives in the same Namespace as the controller. | | `metrics-disable` | `bool` | Disable exposing metrics in the Prometheus format. (default false) | | `metrics-listen-port` | `int` | Sets the port where the Prometheus metrics are exposed. Format: `[1024 - 65535]` (default `9113`) | | `metrics-secure-serving` | `bool` | Configures if the metrics endpoint should be secured using https. Please note that this endpoint will be secured with a self-signed certificate. (default false) | diff --git a/docs/gateway-api-compatibility.md b/docs/gateway-api-compatibility.md index 488ca4ed61..f9984d7d87 100644 --- a/docs/gateway-api-compatibility.md +++ b/docs/gateway-api-compatibility.md @@ -68,7 +68,7 @@ Fields: > Support Levels: > > - Core: Supported. -> - Extended: Not supported. +> - Extended: Partially supported. > - Implementation-specific: Not supported. NGINX Gateway Fabric supports only a single Gateway resource. The Gateway resource must reference NGINX Gateway @@ -91,7 +91,7 @@ Fields: - `allowedRoutes` - supported. - `addresses` - not supported. - `status` - - `addresses` - Pod IPAddress supported. + - `addresses` - partially supported. LoadBalancer and Pod IP. - `conditions` - supported (Condition/Status/Reason): - `Accepted/True/Accepted` - `Accepted/True/ListenersNotValid` diff --git a/docs/installation.md b/docs/installation.md index 6a8d508fe0..c69e5604e6 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -64,6 +64,8 @@ page. ## Expose NGINX Gateway Fabric You can gain access to NGINX Gateway Fabric by creating a `NodePort` Service or a `LoadBalancer` Service. +This Service must live in the same Namespace as the controller. The name of this Service is provided in +the `--service` argument to the controller. > Important > @@ -72,6 +74,9 @@ You can gain access to NGINX Gateway Fabric by creating a `NodePort` Service or > configured for those ports. If you'd like to use different ports in your listeners, > update the manifests accordingly. +NGINX Gateway Fabric will use this Service to set the Addresses field in the Gateway Status resource. A LoadBalancer +Service sets the status field to the IP address and/or Hostname. If no Service exists, the Pod IP address is used. + ### Create a NodePort Service Create a Service with type `NodePort`: diff --git a/internal/framework/controller/predicate/service.go b/internal/framework/controller/predicate/service.go index 9ac8a1b1e4..d1b361fb13 100644 --- a/internal/framework/controller/predicate/service.go +++ b/internal/framework/controller/predicate/service.go @@ -2,7 +2,9 @@ package predicate import ( apiv1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/predicate" ) @@ -63,3 +65,54 @@ func (ServicePortsChangedPredicate) Update(e event.UpdateEvent) bool { return len(newPortSet) > 0 } + +// GatewayServicePredicate implements predicate functions for this Pod's Service. +type GatewayServicePredicate struct { + predicate.Funcs + NSName types.NamespacedName +} + +// Update implements the default UpdateEvent filter for the Gateway Service. +func (gsp GatewayServicePredicate) Update(e event.UpdateEvent) bool { + if e.ObjectOld == nil { + return false + } + if e.ObjectNew == nil { + return false + } + + oldSvc, ok := e.ObjectOld.(*apiv1.Service) + if !ok { + return false + } + + newSvc, ok := e.ObjectNew.(*apiv1.Service) + if !ok { + return false + } + + if client.ObjectKeyFromObject(newSvc) != gsp.NSName { + return false + } + + if oldSvc.Spec.Type != newSvc.Spec.Type { + return true + } + + if newSvc.Spec.Type == apiv1.ServiceTypeLoadBalancer { + oldIngress := oldSvc.Status.LoadBalancer.Ingress + newIngress := newSvc.Status.LoadBalancer.Ingress + + if len(oldIngress) != len(newIngress) { + return true + } + + for i, ingress := range oldIngress { + if ingress.IP != newIngress[i].IP || ingress.Hostname != newIngress[i].Hostname { + return true + } + } + } + + return false +} diff --git a/internal/framework/controller/predicate/service_test.go b/internal/framework/controller/predicate/service_test.go index 85051c94f0..90bda2fd2d 100644 --- a/internal/framework/controller/predicate/service_test.go +++ b/internal/framework/controller/predicate/service_test.go @@ -5,6 +5,8 @@ import ( . "github.com/onsi/gomega" v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/event" @@ -240,7 +242,220 @@ func TestServicePortsChangedPredicate_Update(t *testing.T) { func TestServicePortsChangedPredicate(t *testing.T) { g := NewWithT(t) - p := ServicePortsChangedPredicate{} + p := GatewayServicePredicate{} + + g.Expect(p.Delete(event.DeleteEvent{Object: &v1.Service{}})).To(BeTrue()) + g.Expect(p.Create(event.CreateEvent{Object: &v1.Service{}})).To(BeTrue()) + g.Expect(p.Generic(event.GenericEvent{Object: &v1.Service{}})).To(BeTrue()) +} + +func TestGatewayServicePredicate_Update(t *testing.T) { + testcases := []struct { + objectOld client.Object + objectNew client.Object + msg string + expUpdate bool + }{ + { + msg: "nil objectOld", + objectOld: nil, + objectNew: &v1.Service{}, + expUpdate: false, + }, + { + msg: "nil objectNew", + objectOld: &v1.Service{}, + objectNew: nil, + expUpdate: false, + }, + { + msg: "non-Service objectOld", + objectOld: &v1.Namespace{}, + objectNew: &v1.Service{}, + expUpdate: false, + }, + { + msg: "non-Service objectNew", + objectOld: &v1.Service{}, + objectNew: &v1.Namespace{}, + expUpdate: false, + }, + { + msg: "Service not watched", + objectOld: &v1.Service{}, + objectNew: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "nginx-gateway", + Name: "not-watched", + }, + }, + expUpdate: false, + }, + { + msg: "something irrelevant changed", + objectOld: &v1.Service{ + Spec: v1.ServiceSpec{ + ClusterIP: "1.2.3.4", + }, + }, + objectNew: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "nginx-gateway", + Name: "nginx", + }, + Spec: v1.ServiceSpec{ + ClusterIP: "5.6.7.8", + }, + }, + expUpdate: false, + }, + { + msg: "type changed", + objectOld: &v1.Service{ + Spec: v1.ServiceSpec{ + Type: v1.ServiceTypeLoadBalancer, + }, + }, + objectNew: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "nginx-gateway", + Name: "nginx", + }, + Spec: v1.ServiceSpec{ + Type: v1.ServiceTypeNodePort, + }, + }, + expUpdate: true, + }, + { + msg: "ingress changed length", + objectOld: &v1.Service{ + Spec: v1.ServiceSpec{ + Type: v1.ServiceTypeLoadBalancer, + }, + Status: v1.ServiceStatus{ + LoadBalancer: v1.LoadBalancerStatus{ + Ingress: []v1.LoadBalancerIngress{ + { + IP: "1.2.3.4", + }, + }, + }, + }, + }, + objectNew: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "nginx-gateway", + Name: "nginx", + }, + Spec: v1.ServiceSpec{ + Type: v1.ServiceTypeNodePort, + }, Status: v1.ServiceStatus{ + LoadBalancer: v1.LoadBalancerStatus{ + Ingress: []v1.LoadBalancerIngress{ + { + IP: "1.2.3.4", + }, + { + IP: "5.6.7.8", + }, + }, + }, + }, + }, + expUpdate: true, + }, + { + msg: "IP address changed", + objectOld: &v1.Service{ + Spec: v1.ServiceSpec{ + Type: v1.ServiceTypeLoadBalancer, + }, + Status: v1.ServiceStatus{ + LoadBalancer: v1.LoadBalancerStatus{ + Ingress: []v1.LoadBalancerIngress{ + { + IP: "1.2.3.4", + }, + }, + }, + }, + }, + objectNew: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "nginx-gateway", + Name: "nginx", + }, + Spec: v1.ServiceSpec{ + Type: v1.ServiceTypeNodePort, + }, Status: v1.ServiceStatus{ + LoadBalancer: v1.LoadBalancerStatus{ + Ingress: []v1.LoadBalancerIngress{ + { + IP: "5.6.7.8", + }, + }, + }, + }, + }, + expUpdate: true, + }, + { + msg: "Hostname changed", + objectOld: &v1.Service{ + Spec: v1.ServiceSpec{ + Type: v1.ServiceTypeLoadBalancer, + }, + Status: v1.ServiceStatus{ + LoadBalancer: v1.LoadBalancerStatus{ + Ingress: []v1.LoadBalancerIngress{ + { + Hostname: "one", + }, + }, + }, + }, + }, + objectNew: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "nginx-gateway", + Name: "nginx", + }, + Spec: v1.ServiceSpec{ + Type: v1.ServiceTypeNodePort, + }, Status: v1.ServiceStatus{ + LoadBalancer: v1.LoadBalancerStatus{ + Ingress: []v1.LoadBalancerIngress{ + { + Hostname: "two", + }, + }, + }, + }, + }, + expUpdate: true, + }, + } + + p := GatewayServicePredicate{NSName: types.NamespacedName{Namespace: "nginx-gateway", Name: "nginx"}} + + for _, tc := range testcases { + t.Run(tc.msg, func(t *testing.T) { + g := NewWithT(t) + update := p.Update(event.UpdateEvent{ + ObjectOld: tc.objectOld, + ObjectNew: tc.objectNew, + }) + + g.Expect(update).To(Equal(tc.expUpdate)) + }) + } +} + +func TestGatewayServicePredicate(t *testing.T) { + g := NewWithT(t) + + p := GatewayServicePredicate{} g.Expect(p.Delete(event.DeleteEvent{Object: &v1.Service{}})).To(BeTrue()) g.Expect(p.Create(event.CreateEvent{Object: &v1.Service{}})).To(BeTrue()) diff --git a/internal/framework/status/gateway.go b/internal/framework/status/gateway.go index 366aaa920b..644be890e5 100644 --- a/internal/framework/status/gateway.go +++ b/internal/framework/status/gateway.go @@ -10,7 +10,6 @@ import ( // prepareGatewayStatus prepares the status for a Gateway resource. func prepareGatewayStatus( gatewayStatus GatewayStatus, - podIP string, transitionTime metav1.Time, ) v1beta1.GatewayStatus { listenerStatuses := make([]v1beta1.ListenerStatus, 0, len(gatewayStatus.ListenerStatuses)) @@ -34,15 +33,9 @@ func prepareGatewayStatus( }) } - ipAddrType := v1beta1.IPAddressType - gwPodIP := v1beta1.GatewayStatusAddress{ - Type: &ipAddrType, - Value: podIP, - } - return v1beta1.GatewayStatus{ Listeners: listenerStatuses, - Addresses: []v1beta1.GatewayStatusAddress{gwPodIP}, + Addresses: gatewayStatus.Addresses, Conditions: convertConditions(gatewayStatus.Conditions, gatewayStatus.ObservedGeneration, transitionTime), } } diff --git a/internal/framework/status/gateway_test.go b/internal/framework/status/gateway_test.go index b01832915d..7ff2d23d0f 100644 --- a/internal/framework/status/gateway_test.go +++ b/internal/framework/status/gateway_test.go @@ -12,9 +12,8 @@ import ( ) func TestPrepareGatewayStatus(t *testing.T) { - ipAddrType := v1beta1.IPAddressType podIP := v1beta1.GatewayStatusAddress{ - Type: &ipAddrType, + Type: helpers.GetPointer(v1beta1.IPAddressType), Value: "1.2.3.4", } status := GatewayStatus{ @@ -30,6 +29,7 @@ func TestPrepareGatewayStatus(t *testing.T) { }, }, }, + Addresses: []v1beta1.GatewayStatusAddress{podIP}, ObservedGeneration: 1, } @@ -54,6 +54,6 @@ func TestPrepareGatewayStatus(t *testing.T) { g := NewWithT(t) - result := prepareGatewayStatus(status, "1.2.3.4", transitionTime) + result := prepareGatewayStatus(status, transitionTime) g.Expect(helpers.Diff(expected, result)).To(BeEmpty()) } diff --git a/internal/framework/status/statuses.go b/internal/framework/status/statuses.go index 89284f1661..9af0108aa7 100644 --- a/internal/framework/status/statuses.go +++ b/internal/framework/status/statuses.go @@ -57,8 +57,12 @@ type GatewayStatus struct { ListenerStatuses ListenerStatuses // Conditions is the list of conditions for this Gateway. Conditions []conditions.Condition + // Addresses holds the list of GatewayStatusAddresses. + Addresses []v1beta1.GatewayStatusAddress // ObservedGeneration is the generation of the resource that was processed. ObservedGeneration int64 + // Ignored tells whether or not this Gateway is ignored. + Ignored bool } // ListenerStatus holds the status-related information about a listener in the Gateway resource. diff --git a/internal/framework/status/statusfakes/fake_updater.go b/internal/framework/status/statusfakes/fake_updater.go index 6fcc488c62..6ca5b60c32 100644 --- a/internal/framework/status/statusfakes/fake_updater.go +++ b/internal/framework/status/statusfakes/fake_updater.go @@ -6,6 +6,7 @@ import ( "sync" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status" + "sigs.k8s.io/gateway-api/apis/v1beta1" ) type FakeUpdater struct { @@ -24,6 +25,12 @@ type FakeUpdater struct { arg1 context.Context arg2 status.Status } + UpdateAddressesStub func(context.Context, []v1beta1.GatewayStatusAddress) + updateAddressesMutex sync.RWMutex + updateAddressesArgsForCall []struct { + arg1 context.Context + arg2 []v1beta1.GatewayStatusAddress + } invocations map[string][][]interface{} invocationsMutex sync.RWMutex } @@ -117,6 +124,44 @@ func (fake *FakeUpdater) UpdateArgsForCall(i int) (context.Context, status.Statu return argsForCall.arg1, argsForCall.arg2 } +func (fake *FakeUpdater) UpdateAddresses(arg1 context.Context, arg2 []v1beta1.GatewayStatusAddress) { + var arg2Copy []v1beta1.GatewayStatusAddress + if arg2 != nil { + arg2Copy = make([]v1beta1.GatewayStatusAddress, len(arg2)) + copy(arg2Copy, arg2) + } + fake.updateAddressesMutex.Lock() + fake.updateAddressesArgsForCall = append(fake.updateAddressesArgsForCall, struct { + arg1 context.Context + arg2 []v1beta1.GatewayStatusAddress + }{arg1, arg2Copy}) + stub := fake.UpdateAddressesStub + fake.recordInvocation("UpdateAddresses", []interface{}{arg1, arg2Copy}) + fake.updateAddressesMutex.Unlock() + if stub != nil { + fake.UpdateAddressesStub(arg1, arg2) + } +} + +func (fake *FakeUpdater) UpdateAddressesCallCount() int { + fake.updateAddressesMutex.RLock() + defer fake.updateAddressesMutex.RUnlock() + return len(fake.updateAddressesArgsForCall) +} + +func (fake *FakeUpdater) UpdateAddressesCalls(stub func(context.Context, []v1beta1.GatewayStatusAddress)) { + fake.updateAddressesMutex.Lock() + defer fake.updateAddressesMutex.Unlock() + fake.UpdateAddressesStub = stub +} + +func (fake *FakeUpdater) UpdateAddressesArgsForCall(i int) (context.Context, []v1beta1.GatewayStatusAddress) { + fake.updateAddressesMutex.RLock() + defer fake.updateAddressesMutex.RUnlock() + argsForCall := fake.updateAddressesArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2 +} + func (fake *FakeUpdater) Invocations() map[string][][]interface{} { fake.invocationsMutex.RLock() defer fake.invocationsMutex.RUnlock() @@ -126,6 +171,8 @@ func (fake *FakeUpdater) Invocations() map[string][][]interface{} { defer fake.enableMutex.RUnlock() fake.updateMutex.RLock() defer fake.updateMutex.RUnlock() + fake.updateAddressesMutex.RLock() + defer fake.updateAddressesMutex.RUnlock() copiedInvocations := map[string][][]interface{}{} for key, value := range fake.invocations { copiedInvocations[key] = value diff --git a/internal/framework/status/updater.go b/internal/framework/status/updater.go index d14bebfa47..6dd114b4ea 100644 --- a/internal/framework/status/updater.go +++ b/internal/framework/status/updater.go @@ -16,6 +16,7 @@ import ( ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/controller" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/config" ) //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . Updater @@ -27,6 +28,8 @@ import ( type Updater interface { // Update updates the statuses of the resources. Update(context.Context, Status) + // UpdateAddresses updates the Gateway Addresses when the Gateway Service changes. + UpdateAddresses(context.Context, []v1beta1.GatewayStatusAddress) // Enable enables status updates. The updater will update the statuses in Kubernetes API to ensure they match the // statuses of the last Update invocation. Enable(ctx context.Context) @@ -40,14 +43,14 @@ type UpdaterConfig struct { Client client.Client // Clock is used as a source of time for the LastTransitionTime field in Conditions in resource statuses. Clock Clock + // GatewayPodConfig contains information about this Pod. + GatewayPodConfig config.GatewayPodConfig // Logger holds a logger to be used. Logger logr.Logger // GatewayCtlrName is the name of the Gateway controller. GatewayCtlrName string // GatewayClassName is the name of the GatewayClass resource. GatewayClassName string - // PodIP is the IP address of this Pod. - PodIP string // UpdateGatewayClassStatus enables updating the status of the GatewayClass resource. UpdateGatewayClassStatus bool // LeaderElectionEnabled indicates whether Leader Election is enabled. @@ -200,7 +203,7 @@ func (upd *UpdaterImpl) updateGatewayAPI(ctx context.Context, statuses GatewayAP } upd.writeStatuses(ctx, nsname, &v1beta1.Gateway{}, func(object client.Object) { gw := object.(*v1beta1.Gateway) - gw.Status = prepareGatewayStatus(gs, upd.cfg.PodIP, upd.cfg.Clock.Now()) + gw.Status = prepareGatewayStatus(gs, upd.cfg.Clock.Now()) }) } @@ -250,6 +253,22 @@ func (upd *UpdaterImpl) writeStatuses( } } +// UpdateAddresses is called when the Gateway Status needs its addresses updated. +func (upd *UpdaterImpl) UpdateAddresses(ctx context.Context, addresses []v1beta1.GatewayStatusAddress) { + defer upd.lock.Unlock() + upd.lock.Lock() + + for name, status := range upd.lastStatuses.gatewayAPI.GatewayStatuses { + if status.Ignored { + continue + } + status.Addresses = addresses + upd.lastStatuses.gatewayAPI.GatewayStatuses[name] = status + } + + upd.updateGatewayAPI(ctx, upd.lastStatuses.gatewayAPI) +} + // NewRetryUpdateFunc returns a function which will be used in wait.ExponentialBackoffWithContext. // The function will attempt to Update a kubernetes resource and will be retried in // wait.ExponentialBackoffWithContext if an error occurs. Exported for testing purposes. diff --git a/internal/framework/status/updater_test.go b/internal/framework/status/updater_test.go index 30b8d4127d..66512a9bdc 100644 --- a/internal/framework/status/updater_test.go +++ b/internal/framework/status/updater_test.go @@ -19,6 +19,7 @@ import ( "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status/statusfakes" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/config" staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions" ) @@ -73,9 +74,8 @@ var _ = Describe("Updater", func() { gw, ignoredGw *v1beta1.Gateway hr *v1beta1.HTTPRoute ng *ngfAPI.NginxGateway - ipAddrType = v1beta1.IPAddressType addr = v1beta1.GatewayStatusAddress{ - Type: &ipAddrType, + Type: helpers.GetPointer(v1beta1.IPAddressType), Value: "1.2.3.4", } @@ -97,11 +97,13 @@ var _ = Describe("Updater", func() { SupportedKinds: []v1beta1.RouteGroupKind{{Kind: "HTTPRoute"}}, }, }, + Addresses: []v1beta1.GatewayStatusAddress{addr}, ObservedGeneration: gens.gateways, }, {Namespace: "test", Name: "ignored-gateway"}: { Conditions: staticConds.NewGatewayConflict(), ObservedGeneration: 1, + Ignored: true, }, }, HTTPRouteStatuses: status.HTTPRouteStatuses{ @@ -199,7 +201,6 @@ var _ = Describe("Updater", func() { Message: staticConds.GatewayMessageGatewayConflict, }, }, - Addresses: []v1beta1.GatewayStatusAddress{addr}, }, } } @@ -251,12 +252,14 @@ var _ = Describe("Updater", func() { BeforeAll(func() { updater = status.NewUpdater(status.UpdaterConfig{ - GatewayCtlrName: gatewayCtrlName, - GatewayClassName: gcName, - Client: client, - Logger: zap.New(), - Clock: fakeClock, - PodIP: "1.2.3.4", + GatewayCtlrName: gatewayCtrlName, + GatewayClassName: gcName, + Client: client, + Logger: zap.New(), + Clock: fakeClock, + GatewayPodConfig: config.GatewayPodConfig{ + PodIP: "1.2.3.4", + }, UpdateGatewayClassStatus: true, }) @@ -398,6 +401,38 @@ var _ = Describe("Updater", func() { Expect(helpers.Diff(expectedNG, latestNG)).To(BeEmpty()) }) + When("the Gateway Service is updated with a new address", func() { + AfterEach(func() { + // reset the IP for the remaining tests + updater.UpdateAddresses(context.Background(), []v1beta1.GatewayStatusAddress{ + { + Type: helpers.GetPointer(v1beta1.IPAddressType), + Value: "1.2.3.4", + }, + }) + }) + + It("should update the previous Gateway statuses with new address", func() { + latestGw := &v1beta1.Gateway{} + expectedGw := createExpectedGwWithGeneration(1) + expectedGw.Status.Addresses[0].Value = "5.6.7.8" + + updater.UpdateAddresses(context.Background(), []v1beta1.GatewayStatusAddress{ + { + Type: helpers.GetPointer(v1beta1.IPAddressType), + Value: "5.6.7.8", + }, + }) + + err := client.Get(context.Background(), types.NamespacedName{Namespace: "test", Name: "gateway"}, latestGw) + Expect(err).ToNot(HaveOccurred()) + + expectedGw.ResourceVersion = latestGw.ResourceVersion + + Expect(helpers.Diff(expectedGw, latestGw)).To(BeEmpty()) + }) + }) + It("should not update Gateway API statuses with canceled context - function normally returns", func() { ctx, cancel := context.WithCancel(context.Background()) cancel() @@ -646,9 +681,9 @@ var _ = Describe("Updater", func() { ) Expect(err).ToNot(HaveOccurred()) - // Before this test there were 4 updates to the Gateway resource. - // So now the resource version should equal 24. - Expect(latestGw.ResourceVersion).To(Equal("24")) + // Before this test there were 6 updates to the Gateway resource. + // So now the resource version should equal 26. + Expect(latestGw.ResourceVersion).To(Equal("26")) }) }) }) @@ -661,12 +696,14 @@ var _ = Describe("Updater", func() { BeforeAll(func() { updater = status.NewUpdater(status.UpdaterConfig{ - GatewayCtlrName: gatewayCtrlName, - GatewayClassName: gcName, - Client: client, - Logger: zap.New(), - Clock: fakeClock, - PodIP: "1.2.3.4", + GatewayCtlrName: gatewayCtrlName, + GatewayClassName: gcName, + Client: client, + Logger: zap.New(), + Clock: fakeClock, + GatewayPodConfig: config.GatewayPodConfig{ + PodIP: "1.2.3.4", + }, UpdateGatewayClassStatus: false, }) @@ -710,12 +747,14 @@ var _ = Describe("Updater", func() { Describe("Edge cases", func() { It("panics on update if status type is unknown", func() { updater := status.NewUpdater(status.UpdaterConfig{ - GatewayCtlrName: gatewayCtrlName, - GatewayClassName: gcName, - Client: client, - Logger: zap.New(), - Clock: fakeClock, - PodIP: "1.2.3.4", + GatewayCtlrName: gatewayCtrlName, + GatewayClassName: gcName, + Client: client, + Logger: zap.New(), + Clock: fakeClock, + GatewayPodConfig: config.GatewayPodConfig{ + PodIP: "1.2.3.4", + }, UpdateGatewayClassStatus: true, }) diff --git a/internal/mode/provisioner/handler_test.go b/internal/mode/provisioner/handler_test.go index da61348413..da29a9c1a2 100644 --- a/internal/mode/provisioner/handler_test.go +++ b/internal/mode/provisioner/handler_test.go @@ -60,7 +60,6 @@ var _ = Describe("handler", func() { Logger: zap.New(), GatewayCtlrName: "test.example.com", GatewayClassName: gcName, - PodIP: "1.2.3.4", UpdateGatewayClassStatus: true, }) }) diff --git a/internal/mode/static/build_statuses.go b/internal/mode/static/build_statuses.go index 8a51dd4cbd..2a009f5580 100644 --- a/internal/mode/static/build_statuses.go +++ b/internal/mode/static/build_statuses.go @@ -16,14 +16,18 @@ type nginxReloadResult struct { } // buildGatewayAPIStatuses builds status.Statuses from a Graph. -func buildGatewayAPIStatuses(graph *graph.Graph, nginxReloadRes nginxReloadResult) status.GatewayAPIStatuses { +func buildGatewayAPIStatuses( + graph *graph.Graph, + gwAddresses []v1beta1.GatewayStatusAddress, + nginxReloadRes nginxReloadResult, +) status.GatewayAPIStatuses { statuses := status.GatewayAPIStatuses{ HTTPRouteStatuses: make(status.HTTPRouteStatuses), } statuses.GatewayClassStatuses = buildGatewayClassStatuses(graph.GatewayClass, graph.IgnoredGatewayClasses) - statuses.GatewayStatuses = buildGatewayStatuses(graph.Gateway, graph.IgnoredGateways, nginxReloadRes) + statuses.GatewayStatuses = buildGatewayStatuses(graph.Gateway, graph.IgnoredGateways, gwAddresses, nginxReloadRes) for nsname, r := range graph.Routes { parentStatuses := make([]status.ParentStatus, 0, len(r.ParentRefs)) @@ -105,25 +109,31 @@ func buildGatewayClassStatuses( func buildGatewayStatuses( gateway *graph.Gateway, ignoredGateways map[types.NamespacedName]*v1beta1.Gateway, + gwAddresses []v1beta1.GatewayStatusAddress, nginxReloadRes nginxReloadResult, ) status.GatewayStatuses { statuses := make(status.GatewayStatuses) if gateway != nil { - statuses[client.ObjectKeyFromObject(gateway.Source)] = buildGatewayStatus(gateway, nginxReloadRes) + statuses[client.ObjectKeyFromObject(gateway.Source)] = buildGatewayStatus(gateway, gwAddresses, nginxReloadRes) } for nsname, gw := range ignoredGateways { statuses[nsname] = status.GatewayStatus{ Conditions: staticConds.NewGatewayConflict(), ObservedGeneration: gw.Generation, + Ignored: true, } } return statuses } -func buildGatewayStatus(gateway *graph.Gateway, nginxReloadRes nginxReloadResult) status.GatewayStatus { +func buildGatewayStatus( + gateway *graph.Gateway, + gwAddresses []v1beta1.GatewayStatusAddress, + nginxReloadRes nginxReloadResult, +) status.GatewayStatus { if !gateway.Valid { return status.GatewayStatus{ Conditions: staticConds.DeduplicateConditions(gateway.Conditions), @@ -175,6 +185,7 @@ func buildGatewayStatus(gateway *graph.Gateway, nginxReloadRes nginxReloadResult return status.GatewayStatus{ Conditions: staticConds.DeduplicateConditions(gwConds), ListenerStatuses: listenerStatuses, + Addresses: gwAddresses, ObservedGeneration: gateway.Source.Generation, } } diff --git a/internal/mode/static/build_statuses_test.go b/internal/mode/static/build_statuses_test.go index 77d803840a..74010237be 100644 --- a/internal/mode/static/build_statuses_test.go +++ b/internal/mode/static/build_statuses_test.go @@ -36,6 +36,13 @@ var ( ) func TestBuildStatuses(t *testing.T) { + addr := []v1beta1.GatewayStatusAddress{ + { + Type: helpers.GetPointer(v1beta1.IPAddressType), + Value: "1.2.3.4", + }, + } + invalidRouteCondition := conditions.Condition{ Type: "TestInvalidRoute", Status: metav1.ConditionTrue, @@ -151,11 +158,13 @@ func TestBuildStatuses(t *testing.T) { Conditions: staticConds.NewDefaultListenerConditions(), }, }, + Addresses: addr, ObservedGeneration: 2, }, {Namespace: "test", Name: "ignored-gateway"}: { Conditions: staticConds.NewGatewayConflict(), ObservedGeneration: 1, + Ignored: true, }, }, HTTPRouteStatuses: status.HTTPRouteStatuses{ @@ -196,11 +205,18 @@ func TestBuildStatuses(t *testing.T) { g := NewWithT(t) var nginxReloadRes nginxReloadResult - result := buildGatewayAPIStatuses(graph, nginxReloadRes) + result := buildGatewayAPIStatuses(graph, addr, nginxReloadRes) g.Expect(helpers.Diff(expected, result)).To(BeEmpty()) } func TestBuildStatusesNginxErr(t *testing.T) { + addr := []v1beta1.GatewayStatusAddress{ + { + Type: helpers.GetPointer(v1beta1.IPAddressType), + Value: "1.2.3.4", + }, + } + routes := map[types.NamespacedName]*graph.Route{ {Namespace: "test", Name: "hr-valid"}: { Valid: true, @@ -265,6 +281,7 @@ func TestBuildStatusesNginxErr(t *testing.T) { }, }, }, + Addresses: addr, ObservedGeneration: 2, }, }, @@ -288,7 +305,7 @@ func TestBuildStatusesNginxErr(t *testing.T) { g := NewWithT(t) nginxReloadRes := nginxReloadResult{error: errors.New("test error")} - result := buildGatewayAPIStatuses(graph, nginxReloadRes) + result := buildGatewayAPIStatuses(graph, addr, nginxReloadRes) g.Expect(helpers.Diff(expected, result)).To(BeEmpty()) } @@ -358,6 +375,13 @@ func TestBuildGatewayClassStatuses(t *testing.T) { } func TestBuildGatewayStatuses(t *testing.T) { + addr := []v1beta1.GatewayStatusAddress{ + { + Type: helpers.GetPointer(v1beta1.IPAddressType), + Value: "1.2.3.4", + }, + } + tests := []struct { nginxReloadRes nginxReloadResult gateway *graph.Gateway @@ -387,10 +411,12 @@ func TestBuildGatewayStatuses(t *testing.T) { {Namespace: "test", Name: "ignored-1"}: { Conditions: staticConds.NewGatewayConflict(), ObservedGeneration: 1, + Ignored: true, }, {Namespace: "test", Name: "ignored-2"}: { Conditions: staticConds.NewGatewayConflict(), ObservedGeneration: 2, + Ignored: true, }, }, }, @@ -427,6 +453,7 @@ func TestBuildGatewayStatuses(t *testing.T) { Conditions: staticConds.NewDefaultListenerConditions(), }, }, + Addresses: addr, ObservedGeneration: 2, }, }, @@ -464,6 +491,7 @@ func TestBuildGatewayStatuses(t *testing.T) { Conditions: staticConds.NewListenerUnsupportedValue("unsupported value"), }, }, + Addresses: addr, ObservedGeneration: 2, }, }, @@ -495,6 +523,7 @@ func TestBuildGatewayStatuses(t *testing.T) { Conditions: staticConds.NewListenerUnsupportedValue("unsupported value"), }, }, + Addresses: addr, ObservedGeneration: 2, }, }, @@ -547,6 +576,7 @@ func TestBuildGatewayStatuses(t *testing.T) { }, }, }, + Addresses: addr, ObservedGeneration: 2, }, }, @@ -558,7 +588,7 @@ func TestBuildGatewayStatuses(t *testing.T) { t.Run(test.name, func(t *testing.T) { g := NewWithT(t) - result := buildGatewayStatuses(test.gateway, test.ignoredGateways, test.nginxReloadRes) + result := buildGatewayStatuses(test.gateway, test.ignoredGateways, addr, test.nginxReloadRes) g.Expect(helpers.Diff(test.expected, result)).To(BeEmpty()) }) } diff --git a/internal/mode/static/config/config.go b/internal/mode/static/config/config.go index 0a6910afc7..5bfe6a6244 100644 --- a/internal/mode/static/config/config.go +++ b/internal/mode/static/config/config.go @@ -12,6 +12,8 @@ type Config struct { // GatewayNsName is the namespaced name of a Gateway resource that the Gateway will use. // The Gateway will ignore all other Gateway resources. GatewayNsName *types.NamespacedName + // GatewayPodConfig contains information about this Pod. + GatewayPodConfig GatewayPodConfig // Logger is the Zap Logger used by all components. Logger logr.Logger // GatewayCtlrName is the name of this controller. @@ -20,10 +22,6 @@ type Config struct { ConfigName string // GatewayClassName is the name of the GatewayClass resource that the Gateway will use. GatewayClassName string - // PodIP is the IP address of this Pod. - PodIP string - // Namespace is the Namespace of this Pod. - Namespace string // LeaderElection contains the configuration for leader election. LeaderElection LeaderElection // UpdateGatewayClassStatus enables updating the status of the GatewayClass resource. @@ -34,6 +32,16 @@ type Config struct { HealthConfig HealthConfig } +// GatewayPodConfig contains information about this Pod. +type GatewayPodConfig struct { + // PodIP is the IP address of this Pod. + PodIP string + // ServiceName is the name of the Service that fronts this Pod. + ServiceName string + // Namespace is the namespace of this Pod. + Namespace string +} + // MetricsConfig specifies the metrics config. type MetricsConfig struct { // Port is the port the metrics should be exposed on. diff --git a/internal/mode/static/handler.go b/internal/mode/static/handler.go index 71f24672b4..5ac9812525 100644 --- a/internal/mode/static/handler.go +++ b/internal/mode/static/handler.go @@ -7,15 +7,20 @@ import ( "github.com/go-logr/logr" apiv1 "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/gateway-api/apis/v1beta1" ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/events" + "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status" - "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/config" + ngfConfig "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/config" + ngxConfig "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/file" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/runtime" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state" @@ -30,12 +35,16 @@ type handlerMetricsCollector interface { // eventHandlerConfig holds configuration parameters for eventHandlerImpl. type eventHandlerConfig struct { + // k8sClient is a Kubernetes API client + k8sClient client.Client + // gatewayPodConfig contains information about this Pod. + gatewayPodConfig ngfConfig.GatewayPodConfig // processor is the state ChangeProcessor. processor state.ChangeProcessor // serviceResolver resolves Services to Endpoints. serviceResolver resolver.ServiceResolver // generator is the nginx config generator. - generator config.Generator + generator ngxConfig.Generator // nginxFileMgr is the file Manager for nginx. nginxFileMgr file.Manager // nginxRuntimeMgr manages nginx runtime. @@ -86,22 +95,7 @@ func (h *eventHandlerImpl) HandleEventBatch(ctx context.Context, logger logr.Log }() for _, event := range batch { - switch e := event.(type) { - case *events.UpsertEvent: - if cfg, ok := e.Resource.(*ngfAPI.NginxGateway); ok { - h.updateControlPlaneAndSetStatus(ctx, logger, cfg) - } else { - h.cfg.processor.CaptureUpsertChange(e.Resource) - } - case *events.DeleteEvent: - if _, ok := e.Type.(*ngfAPI.NginxGateway); ok { - h.updateControlPlaneAndSetStatus(ctx, logger, nil) - } else { - h.cfg.processor.CaptureDeleteChange(e.Type, e.NamespacedName) - } - default: - panic(fmt.Errorf("unknown event type %T", e)) - } + h.handleEvent(ctx, logger, event) } changed, graph := h.cfg.processor.Process() @@ -131,7 +125,53 @@ func (h *eventHandlerImpl) HandleEventBatch(ctx context.Context, logger logr.Log } } - h.cfg.statusUpdater.Update(ctx, buildGatewayAPIStatuses(graph, nginxReloadRes)) + gwAddresses, err := getGatewayAddresses(ctx, h.cfg.k8sClient, nil, h.cfg.gatewayPodConfig) + if err != nil { + logger.Error(err, "Setting GatewayStatusAddress to Pod IP Address") + } + + h.cfg.statusUpdater.Update(ctx, buildGatewayAPIStatuses(graph, gwAddresses, nginxReloadRes)) +} + +func (h *eventHandlerImpl) handleEvent(ctx context.Context, logger logr.Logger, event interface{}) { + switch e := event.(type) { + case *events.UpsertEvent: + switch obj := e.Resource.(type) { + case *ngfAPI.NginxGateway: + h.updateControlPlaneAndSetStatus(ctx, logger, obj) + case *apiv1.Service: + podConfig := h.cfg.gatewayPodConfig + if obj.Name == podConfig.ServiceName && obj.Namespace == podConfig.Namespace { + gwAddresses, err := getGatewayAddresses(ctx, h.cfg.k8sClient, obj, h.cfg.gatewayPodConfig) + if err != nil { + logger.Error(err, "Setting GatewayStatusAddress to Pod IP Address") + } + h.cfg.statusUpdater.UpdateAddresses(ctx, gwAddresses) + } + h.cfg.processor.CaptureUpsertChange(e.Resource) + default: + h.cfg.processor.CaptureUpsertChange(e.Resource) + } + case *events.DeleteEvent: + switch e.Type.(type) { + case *ngfAPI.NginxGateway: + h.updateControlPlaneAndSetStatus(ctx, logger, nil) + case *apiv1.Service: + podConfig := h.cfg.gatewayPodConfig + if e.NamespacedName.Name == podConfig.ServiceName && e.NamespacedName.Namespace == podConfig.Namespace { + gwAddresses, err := getGatewayAddresses(ctx, h.cfg.k8sClient, nil, h.cfg.gatewayPodConfig) + if err != nil { + logger.Error(err, "Setting GatewayStatusAddress to Pod IP Address") + } + h.cfg.statusUpdater.UpdateAddresses(ctx, gwAddresses) + } + h.cfg.processor.CaptureDeleteChange(e.Type, e.NamespacedName) + default: + h.cfg.processor.CaptureDeleteChange(e.Type, e.NamespacedName) + } + default: + panic(fmt.Errorf("unknown event type %T", e)) + } } func (h *eventHandlerImpl) updateNginx(ctx context.Context, conf dataplane.Configuration) error { @@ -188,3 +228,59 @@ func (h *eventHandlerImpl) updateControlPlaneAndSetStatus( logger.Info("Reconfigured control plane.") } } + +// getGatewayAddresses gets the addresses for the Gateway. +func getGatewayAddresses( + ctx context.Context, + k8sClient client.Client, + svc *v1.Service, + podConfig config.GatewayPodConfig, +) ([]v1beta1.GatewayStatusAddress, error) { + podAddress := []v1beta1.GatewayStatusAddress{ + { + Type: helpers.GetPointer(v1beta1.IPAddressType), + Value: podConfig.PodIP, + }, + } + + var gwSvc v1.Service + if svc == nil { + key := types.NamespacedName{Name: podConfig.ServiceName, Namespace: podConfig.Namespace} + if err := k8sClient.Get(ctx, key, &gwSvc); err != nil { + return podAddress, fmt.Errorf("error finding Service for Gateway: %w", err) + } + } else { + gwSvc = *svc + } + + var addresses, hostnames []string + switch gwSvc.Spec.Type { + case v1.ServiceTypeLoadBalancer: + for _, ingress := range gwSvc.Status.LoadBalancer.Ingress { + if ingress.IP != "" { + addresses = append(addresses, ingress.IP) + } else if ingress.Hostname != "" { + hostnames = append(hostnames, ingress.Hostname) + } + } + } + + gwAddresses := make([]v1beta1.GatewayStatusAddress, 0, len(addresses)+len(hostnames)) + for _, addr := range addresses { + statusAddr := v1beta1.GatewayStatusAddress{ + Type: helpers.GetPointer(v1beta1.IPAddressType), + Value: addr, + } + gwAddresses = append(gwAddresses, statusAddr) + } + + for _, hostname := range hostnames { + statusAddr := v1beta1.GatewayStatusAddress{ + Type: helpers.GetPointer(v1beta1.HostnameAddressType), + Value: hostname, + } + gwAddresses = append(gwAddresses, statusAddr) + } + + return gwAddresses, nil +} diff --git a/internal/mode/static/handler_test.go b/internal/mode/static/handler_test.go index 94aad2fb40..d5f3b5377c 100644 --- a/internal/mode/static/handler_test.go +++ b/internal/mode/static/handler_test.go @@ -7,9 +7,11 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "go.uber.org/zap" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" + "sigs.k8s.io/controller-runtime/pkg/client/fake" ctlrZap "sigs.k8s.io/controller-runtime/pkg/log/zap" "sigs.k8s.io/gateway-api/apis/v1beta1" @@ -19,6 +21,7 @@ import ( "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status/statusfakes" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/config" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/metrics/collectors" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/configfakes" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/file" @@ -67,6 +70,7 @@ var _ = Describe("eventHandler", func() { fakeEventRecorder = record.NewFakeRecorder(1) handler = newEventHandlerImpl(eventHandlerConfig{ + k8sClient: fake.NewFakeClient(), processor: fakeProcessor, generator: fakeGenerator, logLevelSetter: newZapLogLevelSetter(zap.NewAtomicLevel()), @@ -76,7 +80,11 @@ var _ = Describe("eventHandler", func() { eventRecorder: fakeEventRecorder, healthChecker: &healthChecker{}, controlConfigNSName: types.NamespacedName{Namespace: namespace, Name: configName}, - metricsCollector: collectors.NewControllerNoopCollector(), + gatewayPodConfig: config.GatewayPodConfig{ + ServiceName: "nginx-gateway", + Namespace: "nginx-gateway", + }, + metricsCollector: collectors.NewControllerNoopCollector(), }) Expect(handler.cfg.healthChecker.ready).To(BeFalse()) }) @@ -221,6 +229,55 @@ var _ = Describe("eventHandler", func() { }) }) + When("receiving Service updates", func() { + It("should not call UpdateAddresses if the Service is not for the Gateway Pod", func() { + e := &events.UpsertEvent{Resource: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "not-nginx-gateway", + }, + }} + batch := []interface{}{e} + + handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) + + Expect(fakeStatusUpdater.UpdateAddressesCallCount()).To(BeZero()) + + de := &events.DeleteEvent{Type: &v1.Service{}} + batch = []interface{}{de} + + handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) + + Expect(fakeStatusUpdater.UpdateAddressesCallCount()).To(BeZero()) + }) + + It("should update the addresses when the Gateway Service is upserted", func() { + e := &events.UpsertEvent{Resource: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nginx-gateway", + Namespace: "nginx-gateway", + }, + }} + batch := []interface{}{e} + + handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) + Expect(fakeStatusUpdater.UpdateAddressesCallCount()).ToNot(BeZero()) + }) + + It("should update the addresses when the Gateway Service is deleted", func() { + e := &events.DeleteEvent{ + Type: &v1.Service{}, + NamespacedName: types.NamespacedName{ + Name: "nginx-gateway", + Namespace: "nginx-gateway", + }, + } + batch := []interface{}{e} + + handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) + Expect(fakeStatusUpdater.UpdateAddressesCallCount()).ToNot(BeZero()) + }) + }) + It("should set the health checker status properly when there are changes", func() { e := &events.UpsertEvent{Resource: &v1beta1.HTTPRoute{}} batch := []interface{}{e} @@ -279,3 +336,51 @@ var _ = Describe("eventHandler", func() { Expect(handle).Should(Panic()) }) }) + +var _ = Describe("getGatewayAddresses", func() { + It("gets gateway addresses from a Service", func() { + fakeClient := fake.NewFakeClient() + podConfig := config.GatewayPodConfig{ + PodIP: "1.2.3.4", + ServiceName: "my-service", + Namespace: "nginx-gateway", + } + + // no Service exists yet, should get error and Pod Address + addrs, err := getGatewayAddresses(context.Background(), fakeClient, nil, podConfig) + Expect(err).To(HaveOccurred()) + Expect(addrs).To(HaveLen(1)) + Expect(addrs[0].Value).To(Equal("1.2.3.4")) + + // Create LoadBalancer Service + svc := v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-service", + Namespace: "nginx-gateway", + }, + Spec: v1.ServiceSpec{ + Type: v1.ServiceTypeLoadBalancer, + }, + Status: v1.ServiceStatus{ + LoadBalancer: v1.LoadBalancerStatus{ + Ingress: []v1.LoadBalancerIngress{ + { + IP: "34.35.36.37", + }, + { + Hostname: "myhost", + }, + }, + }, + }, + } + + Expect(fakeClient.Create(context.Background(), &svc)).To(Succeed()) + + addrs, err = getGatewayAddresses(context.Background(), fakeClient, &svc, podConfig) + Expect(err).ToNot(HaveOccurred()) + Expect(addrs).To(HaveLen(2)) + Expect(addrs[0].Value).To(Equal("34.35.36.37")) + Expect(addrs[1].Value).To(Equal("myhost")) + }) +}) diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go index 90ec48275b..7a04f33e8d 100644 --- a/internal/mode/static/manager.go +++ b/internal/mode/static/manager.go @@ -90,7 +90,7 @@ func StartManager(cfg config.Config) error { ctx := ctlr.SetupSignalHandler() controlConfigNSName := types.NamespacedName{ - Namespace: cfg.Namespace, + Namespace: cfg.GatewayPodConfig.Namespace, Name: cfg.ConfigName, } if err := registerControllers(ctx, cfg, mgr, recorder, logLevelSetter, eventCh, controlConfigNSName); err != nil { @@ -157,14 +157,15 @@ func StartManager(cfg config.Config) error { GatewayCtlrName: cfg.GatewayCtlrName, GatewayClassName: cfg.GatewayClassName, Client: mgr.GetClient(), - PodIP: cfg.PodIP, Logger: cfg.Logger.WithName("statusUpdater"), Clock: status.NewRealClock(), UpdateGatewayClassStatus: cfg.UpdateGatewayClassStatus, LeaderElectionEnabled: cfg.LeaderElection.Enabled, + GatewayPodConfig: cfg.GatewayPodConfig, }) eventHandler := newEventHandlerImpl(eventHandlerConfig{ + k8sClient: mgr.GetClient(), processor: processor, serviceResolver: resolver.NewServiceResolverImpl(mgr.GetClient()), generator: ngxcfg.NewGeneratorImpl(), @@ -178,6 +179,7 @@ func StartManager(cfg config.Config) error { eventRecorder: recorder, healthChecker: hc, controlConfigNSName: controlConfigNSName, + gatewayPodConfig: cfg.GatewayPodConfig, metricsCollector: handlerCollector, }) @@ -208,7 +210,7 @@ func StartManager(cfg config.Config) error { leaderElectorLogger.Info("Stopped leading") statusUpdater.Disable() }, - lockNs: cfg.Namespace, + lockNs: cfg.GatewayPodConfig.Namespace, lockName: cfg.LeaderElection.LockName, identity: cfg.LeaderElection.Identity, }) @@ -268,6 +270,15 @@ func registerControllers( controller.WithK8sPredicate(predicate.ServicePortsChangedPredicate{}), }, }, + { + objectType: &apiv1.Service{}, + options: func() []controller.Option { + svcNSName := types.NamespacedName{Namespace: cfg.GatewayPodConfig.Namespace, Name: cfg.GatewayPodConfig.ServiceName} + return []controller.Option{ + controller.WithK8sPredicate(predicate.GatewayServicePredicate{NSName: svcNSName}), + } + }(), + }, { objectType: &apiv1.Secret{}, },