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

Add cache plugin to coredns #38

Merged
merged 1 commit into from
Aug 18, 2021

Conversation

Bolodya1997
Copy link

Adds cache plugin to coredns.

Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
@edwarnicke
Copy link
Member

I like DNS caching a lot... but am curious what triggered adding this :)

@Bolodya1997
Copy link
Author

Bolodya1997 commented Aug 18, 2021

I like DNS caching a lot... but am curious what triggered adding this :)

Looks like ping have some timeout to make DNS resolving - tried locally with setting 10ms latency between kind nodes and it always fails. And it looks like sometimes it is a problem when we testing it on the real clouds.
So we can start using nslookup instead of ping in our tests, but it probably wouldn't look so good. The other solution we can try is to add cache plugin to fix issue with latency between the Client and the Endpoint nodes.

@denis-tingaikin
Copy link
Member

denis-tingaikin commented Aug 18, 2021

@edwarnicke Yes, I suggested to use the cache to avoid some unexpected issues that @Bolodya1997 found(#38 (comment)) during testing failed build on CI. As side effect it could a bit improve performance for DNS NSM scenarios :)

@denis-tingaikin
Copy link
Member

@edwarnicke Do you have any additional comments on this PR?

@edwarnicke
Copy link
Member

I am a huge fan of the cache... thank you for explaining what was going on :)

@edwarnicke edwarnicke merged commit 6d49839 into networkservicemesh:master Aug 18, 2021
@denis-tingaikin denis-tingaikin added this to In progress in Issue/PR tracking via automation Aug 18, 2021
@denis-tingaikin denis-tingaikin moved this from In progress to Recently merged PRs in Issue/PR tracking Aug 18, 2021
@denis-tingaikin denis-tingaikin moved this from Recently merged PRs to Done in Issue/PR tracking Oct 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants