Skip to content

Commit

Permalink
Set Service address in Gateway Status (#1141)
Browse files Browse the repository at this point in the history
Problem: NGF would only set the Gateway Status to the IP address of the NGF Pod. However, a Service will generally be the entrypoint, so we need to set that address in the status for application developers.

Solution: Using a CLI argument to identify the Service that is linked to this NGF Pod, we will get the Service and set the Addresses accordingly. LoadBalancer Service will set to the ingress IP and/or Hostname. If no Service exists or there is an error, then the Pod IP is used.

We also shouldn't deploy a Service for conformance tests now, since they could be run in an environment where NodePort doesn't work (like kind). These tests will use the Pod IP.
  • Loading branch information
sjberman authored Oct 17, 2023
1 parent cd8ba03 commit f0bae36
Show file tree
Hide file tree
Showing 24 changed files with 748 additions and 80 deletions.
18 changes: 16 additions & 2 deletions cmd/gateway/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -86,6 +87,9 @@ func createStaticModeCommand() *cobra.Command {
configName = stringValidatingValue{
validator: validateResourceName,
}
serviceName = stringValidatingValue{
validator: validateResourceName,
}
disableMetrics bool
metricsSecure bool
metricsListenPort = intValidatingValue{
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
17 changes: 17 additions & 0 deletions cmd/gateway/commands_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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{
Expand Down
2 changes: 0 additions & 2 deletions conformance/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions conformance/provisioner/static-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions deploy/helm-chart/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
1 change: 1 addition & 0 deletions deploy/manifests/nginx-gateway.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions docs/cli-help.md
Original file line number Diff line number Diff line change
Expand Up @@ -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) |
Expand Down
4 changes: 2 additions & 2 deletions docs/gateway-api-compatibility.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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`
Expand Down
5 changes: 5 additions & 0 deletions docs/installation.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
>
Expand All @@ -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`:
Expand Down
53 changes: 53 additions & 0 deletions internal/framework/controller/predicate/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
}
Loading

0 comments on commit f0bae36

Please sign in to comment.