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

refactor(dns): remove unnecessary DNS client initialization #13479

Merged
merged 11 commits into from
Aug 19, 2024
Merged

Conversation

ProBrian
Copy link
Contributor

@ProBrian ProBrian commented Aug 9, 2024

Summary

There are muliple duplcated dns initialization which is unnecessary, This PR aims to improve the logic of dns initialization which as less as possible.

Checklist

  • The Pull Request has tests
  • A changelog file has been created under changelog/unreleased/kong or skip-changelog label added on PR if changelog is unnecessary. README.md
  • There is a user-facing docs PR against https://github.com/Kong/docs.konghq.com - PUT DOCS PR HERE

Issue reference

Fix KAG-5059

@CLAassistant
Copy link

CLAassistant commented Aug 9, 2024

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added core/balancer cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee labels Aug 9, 2024
@ProBrian ProBrian requested a review from chobits August 9, 2024 06:57
@ProBrian ProBrian requested a review from ADD-SP August 9, 2024 07:46
Copy link
Contributor Author

@ProBrian ProBrian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dns init is done in kong/conf_loader/init.lua:

assert(require("kong.tools.dns")(conf))

From this point on, we can just use the initialized dns client which is already loaded.

kong/runloop/balancer/targets.lua Show resolved Hide resolved
kong/timing/hooks/dns.lua Outdated Show resolved Hide resolved
kong/init.lua Show resolved Hide resolved
@outsinre
Copy link
Contributor

I wonder if we can update this https://github.com/Kong/kong-ee/blob/8157e937419695f142fad22e8d76fd98de074876/kong/globalpatches.lua#L536 to something like

client = require("kong.tools.dns")(kong and kong.configuration)

@chronolaw chronolaw changed the title chore(dns): remove unnecessary dns initiazion refactor(dns): remove unnecessary dns initialization Aug 12, 2024
@ProBrian
Copy link
Contributor Author

I wonder if we can update this https://github.com/Kong/kong-ee/blob/8157e937419695f142fad22e8d76fd98de074876/kong/globalpatches.lua#L536 to something like

client = require("kong.tools.dns")(kong and kong.configuration)

Seems kong.configuration is not yet init when global patch module first required, so kong and kong.configuration will not be applied, what do you think @chobits ?

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

decK integration tests

⚠️ failure detected. Please check the workflow logs for more details.

@chobits chobits changed the title refactor(dns): remove unnecessary dns initialization refactor(dns): remove unnecessary DNS client initialization Aug 19, 2024
@ADD-SP ADD-SP merged commit 3cff1b1 into master Aug 19, 2024
28 checks passed
@ADD-SP ADD-SP deleted the chore/dns_init branch August 19, 2024 08:24
@team-gateway-bot
Copy link
Collaborator

Reviews may only be requested from collaborators. One or more of the users or teams you specified is not a collaborator of the Kong/kong-ee repository.

@github-actions github-actions bot added the incomplete-cherry-pick A cherry-pick was incomplete and needs manual intervention label Aug 19, 2024
@Kong Kong deleted a comment from team-gateway-bot Aug 19, 2024
@kikito kikito removed the incomplete-cherry-pick A cherry-pick was incomplete and needs manual intervention label Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee chore Not part of the core functionality of kong, but still needed core/admin-api core/balancer size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants