Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set Service address in Gateway Status #1141

Merged
merged 4 commits into from
Oct 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.`,
sjberman marked this conversation as resolved.
Show resolved Hide resolved
)

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 {
kate-osborn marked this conversation as resolved.
Show resolved Hide resolved
if ingress.IP != newIngress[i].IP || ingress.Hostname != newIngress[i].Hostname {
return true
}
}
}

return false
}
Loading
Loading