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

Conversation

sjberman
Copy link
Contributor

@sjberman sjberman commented Oct 13, 2023

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.

Testing: Verified LoadBalancer Service in GKE cluster. Verified NodePort Service and Pod IP in local kind cluster.

Closes #604

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

@sjberman sjberman requested a review from a team as a code owner October 13, 2023 18:17
@github-actions github-actions bot added documentation Improvements or additions to documentation enhancement New feature or request labels Oct 13, 2023
cmd/gateway/commands.go Show resolved Hide resolved
cmd/gateway/commands.go Outdated Show resolved Hide resolved
deploy/helm-chart/templates/deployment.yaml Outdated Show resolved Hide resolved
internal/framework/controller/predicate/service_test.go Outdated Show resolved Hide resolved
internal/mode/static/manager.go Outdated Show resolved Hide resolved
internal/framework/status/gateway.go Outdated Show resolved Hide resolved
internal/mode/static/handler.go Outdated Show resolved Hide resolved
internal/framework/status/gateway.go Outdated Show resolved Hide resolved
internal/framework/status/gateway.go Outdated Show resolved Hide resolved
internal/framework/status/gateway.go Outdated Show resolved Hide resolved
docs/gateway-api-compatibility.md Outdated Show resolved Hide resolved
internal/framework/status/gateway.go Outdated Show resolved Hide resolved
internal/framework/status/gateway.go Outdated Show resolved Hide resolved
internal/framework/status/updater.go Show resolved Hide resolved
@sjberman sjberman force-pushed the enh/gateway-address branch from 59b4e80 to 466771b Compare October 16, 2023 19:04
internal/framework/status/updater.go Outdated Show resolved Hide resolved
internal/mode/static/handler.go Show resolved Hide resolved
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.
@sjberman sjberman force-pushed the enh/gateway-address branch from 466771b to a9f6192 Compare October 17, 2023 19:59
@sjberman sjberman merged commit f0bae36 into nginxinc:main Oct 17, 2023
22 checks passed
@sjberman sjberman deleted the enh/gateway-address branch October 17, 2023 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Support NodePort and LoadBalancer Service Types for GatewayStatus Addresses field
4 participants