Pass k8s Service data through to the TCP balancer script.
#3615
Conversation
k8s-ci-robot
added
cncf-cla: yes
Fixes broken L4 ExternalName services. Details --------- The `tcp_udp_balancer.lua` script checks if the property `backend.service.spec["type"]` equals "ExternalName". If so, the script does a DNS lookup on the name in order to configure the backend configuration. However, before this commit, the k8s `Service` data was _not_ set on the `Backend` struct passed into the `tcp_udp_balancer.lua` script and therefore the ExternalName check always returned false. This commit fixes the issue by setting the `Service` field on the `Backend` struct. This also requires adding a new field to the `L4Backend` struct first, so that it's available to set on the `Backend`.
kppullin
force-pushed
the
fix-l4-external-names
branch
from
f7a4558 to
f0173f0
Compare
internal/ingress/controller/nginx.go
Outdated
| @@ -829,6 +830,7 @@ func configureDynamically(pcfg *ingress.Configuration, port int, isDynamicCertif | |||
| Name: key, | |||
| Endpoints: ep.Endpoints, | |||
| Port: intstr.FromInt(ep.Port), | |||
| Service: ep.Service, | |||
There was a problem hiding this comment.
Please check https://github.com/kubernetes/ingress-nginx/pull/3615/files#diff-cde3fffe2425ad7efaa8add1d05ae2c0R784. We should not send a full service object.
There was a problem hiding this comment.
Thanks for the callout @ElvinEfendi. I've addressed this in the latest commit.
k8s-ci-robot
added
size/S
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ElvinEfendi, kppullin The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
|
Thanks @kppullin ! Would be more complete to create an e2e test for this to make sure ExternalName works in stream mode. |
What this PR does / why we need it:
Fixes broken L4 ExternalName services.
The
tcp_udp_balancer.luascript checks if the propertybackend.service.spec["type"]equals"ExternalName". If so,the script does a DNS lookup on the name in order to configure
the backend configuration.
However, before this commit, the k8s
Servicedata wasnot set on the
Backendstruct passed into thetcp_udp_balancer.luascript and therefore the
ExternalNamecheck always returned false.This commit fixes the issue by setting the
Servicefield onthe
Backendstruct. This also requires adding a new field to theL4Backendstruct, so that it's available to set on theBackend.