Fix DNS lookup failures in L4 services #3775
Conversation
k8s-ci-robot
added
cncf-cla: yes
| Update(svc) | ||
| Expect(err).NotTo(HaveOccurred(), "unexpected error updating service") | ||
|
|
||
| // Update the TCP configmap to link port 5353 to the DNS external name service |
There was a problem hiding this comment.
Can you use UpdateNginxConfigMapData helper function instead?
There was a problem hiding this comment.
I took a look at UpdateNginxConfigMapData, however it appears hard coded to update the nginx-configuration configmap (copied below). This bit of test code here needs to update the tcp-services configmap. Would you suggest extending UpdateNginxConfigMapData to accept a parameter with the configmap name or leave as is?
config, err := f.KubeClientSet.
CoreV1().
ConfigMaps(f.IngressController.Namespace).
Get("nginx-configuration", metav1.GetOptions{})
There was a problem hiding this comment.
You are right, I missed that.
| configuration = res | ||
| configuration.nameservers = { {{ buildResolversForLua $cfg.Resolver $cfg.DisableIpv6DNS }} } | ||
| end | ||
|
|
There was a problem hiding this comment.
Not the concern of this PR, but we should really have just configuration.lua. This applies to other stream subsyste Lu code duplication as well.
There's an interface ngx.config.subsystem available where we can dynamically check whether it is http or stream subsystem when the module loads. An example: https://github.com/openresty/lua-resty-core/blob/b4c6bd2554a6a27dbca5d637fb18be9a54ec61f6/lib/resty/core/base.lua#L18
|
/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
|
Thank you @ElvinEfendi! |
What this PR does / why we need it:
While adding an e2e test for #3615 (comment) I ran into DNS lookup failures when resolving DNS for external name TCP & UDP services. Any external name TCP & UDP services defined with a hostname (vs raw IP address) do not work due to the failing DNS lookups.
This PR fixes these DNS failures.
Details
Before this change,
nginx.tmplset thenameserversvariable ontcp_udp_configuration.lua. However, no code references this variable. Instead, bothbalancer.luaandtcp_udp_balancer.lualoaddns.lua, anddns.luaalways referencesconfiguration.nameservers.This PR removes the
nameserversvariable fromtcp_udp_configuration, updates the nginx template'sstreamconfig to initialize thenameserversvariable onconfiguration.lua, and adds an e2e test to validate a L4 TCP external name service by performing a DNS lookup on google's nameservers.