feat: add support for passing resolv.conf in dns discovery #9770
Conversation
Signed-off-by: revolyssup <ashishjaitiwari15112000@gmail.com>
Signed-off-by: revolyssup <ashishjaitiwari15112000@gmail.com>
Signed-off-by: revolyssup <ashishjaitiwari15112000@gmail.com>
Signed-off-by: revolyssup <ashishjaitiwari15112000@gmail.com>
|
@monkeyDluffy6017 Is this feature desirable? |
t/discovery/dns/mix.t
Outdated
| --- grep_error_log eval | ||
| qr/connect to 127.0.0.1:1053/ | ||
| --- grep_error_log_out | ||
| connect to 127.0.0.1:1053 | ||
| connect to 127.0.0.1:1053 | ||
| connect to 127.0.0.1:1053 | ||
| connect to 127.0.0.1:1053 | ||
| connect to 127.0.0.1:1053 |
There was a problem hiding this comment.
Why do you delete this?
There was a problem hiding this comment.
Must have been done by mistake while testing something. Pushed the latest commit
Signed-off-by: revolyssup <ashishjaitiwari15112000@gmail.com>
apisix/discovery/dns/schema.lua
Outdated
| @@ -24,6 +24,9 @@ return { | |||
| type = "string", | |||
| }, | |||
| }, | |||
| resolvconf = { | |||
There was a problem hiding this comment.
We should add this to config-default.yaml
There was a problem hiding this comment.
config-default already has servers
apisix/conf/config-default.yaml
Line 279 in 01763b5
and only one of
servers or resolvconf is needed. Should I add it anyways?
There was a problem hiding this comment.
You should add the resolv_conf to the config-default.yaml, or nobody will find this attribute, add comments to clarify that servers or resolvconf is mutual exclusive
There was a problem hiding this comment.
Okay sounds good. Adding it..
apisix/discovery/dns/init.lua
Outdated
| local default_order = {"last", "SRV", "A", "AAAA", "CNAME"} | ||
| local order = core.table.try_read_attr(local_conf, "discovery", "dns", "order") | ||
| order = order or default_order | ||
|
|
||
| local opts = { | ||
| hosts = {}, | ||
| resolvConf = {}, | ||
| resolvConf = resolvconf, |
There was a problem hiding this comment.
Use resolv_conf to replace all variables
There was a problem hiding this comment.
okay except for the resolveConf key inside opts because that field is in the lua-resty-dns.
Signed-off-by: revolyssup <ashishjaitiwari15112000@gmail.com>
t/discovery/dns/resolv.conf
Outdated
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
| # | ||
| nameserver 127.0.0.1:1053 |
There was a problem hiding this comment.
Maybe you could create the file in build-cache dynamically like the function set_coredns
monkeyDluffy6017
added
wait for update
Signed-off-by: Ashish Tiwari <ashishjaitiwari15112000@gmail.com>
Signed-off-by: Ashish Tiwari <ashishjaitiwari15112000@gmail.com>
|
Please make the ci pass |
Signed-off-by: Ashish Tiwari <ashishjaitiwari15112000@gmail.com>
|
@monkeyDluffy6017 Tests passing now |
Revolyssup
removed
the
wait for update
ci/common.sh
Outdated
| @@ -140,6 +140,10 @@ set_coredns() { | |||
| pushd t/coredns || exit 1 | |||
| ../../build-cache/coredns -dns.port=1053 & | |||
| popd || exit 1 | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
Remove redundant blank
| @@ -128,3 +128,4 @@ connect to 127.0.0.1:1053 | |||
| connect to 127.0.0.1:1053 | |||
| connect to 127.0.0.1:1053 | |||
| connect to 127.0.0.1:1053 | |||
| connect to 127.0.0.1:1053 | |||
There was a problem hiding this comment.
Why do you add this? it doesn't look like it has anything to do with your modifications
There was a problem hiding this comment.
earlier it was trying 4 times but when resolv.conf is used it tries one more times(5 times)
Signed-off-by: Ashish Tiwari <ashishjaitiwari15112000@gmail.com>
Description
Fixes #9565
Checklist