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

Coredns plugin #403

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Coredns plugin #403

wants to merge 5 commits into from

Conversation

mikenairn
Copy link
Member

No description provided.

mikenairn and others added 2 commits March 7, 2025 15:05
Adds a new coredns plugin with a "kuadrant" directive.

Signed-off-by: Michael Nairn <[email protected]>
@@ -0,0 +1,2 @@
# kuadrant CoreDNS Plugin

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we expect others might want to make use of this in their own core dns? Should we add more details and consider making it available to others to use?

Copy link
Member Author

Choose a reason for hiding this comment

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

I intend to add some content to this README before i move it from draft, but in terms of someone wanting to include this plugin in their own CoreDNS then it would be done in the same way as any other external plugin as described here.

@mikenairn mikenairn force-pushed the coredns_plugin branch 4 times, most recently from 0e306b9 to fbe4693 Compare March 10, 2025 11:53
* Update example Corefie and remove file directive
* Move the db-generator under plugin so the demo db can more easily be
  built and embedded in the coredns image.
* Add make target to generate demo db `make
  coredns-generate-demo-geo-db`.
* Fix imports and code formatting.

Signed-off-by: Michael Nairn <[email protected]>
Adds a anew buid images github action for the kuadrant CoreDNS plugin.

Add coredns_plugin to branches (DO NOT MERGE)

Signed-off-by: Michael Nairn <[email protected]>
var answer *dns.RR
var geoRRs []geodRR

roundRobinShuffle(grrs)
Copy link
Member Author

Choose a reason for hiding this comment

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

This should probably be removed, but it makes me wonder what we would like the default behaviour to be when we can't match any geo to the current request. Do we want it to always just return the first RR in the list?

Currently this will cause it to randomise what it would return if there are multiple geos available.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should leverage the default GEO like AWS

for _, name := range dnsserver.Directives {

if dropPlugins[name] {
if !alreadyAdded {
Copy link
Contributor

Choose a reason for hiding this comment

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

This reads a bit odd? Like it should be outside of the if?

},
},
&v1alpha1.DNSRecord{},
defaultResyncPeriod,
Copy link
Contributor

Choose a reason for hiding this comment

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

does it matter that we are passing 0 as default? Not sure what that means

@mikenairn mikenairn force-pushed the coredns_plugin branch 2 times, most recently from 6a7f364 to a9b1b88 Compare March 10, 2025 18:11
Limit the number of plugins included by default in the kuadrant build of
CoreDNS. The plugins enabled are the required ones and more simple
plugins with few additional dependencies but are useful for
debug/testing purposes.

The `trace` plugin had licensing issues and was the main reason to
reduce the plugin imports.

Signed-off-by: Michael Nairn <[email protected]>
sort.Slice(address, func(i, j int) bool {
return address[i].weight > address[j].weight
})
v := w.randUint(wsum)
Copy link
Contributor

Choose a reason for hiding this comment

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

So generates a random number between 0 and the sum of all the weights. Then orders the addresses by weight desc and returns the first result that has a weight or combined weight greater than the random value. So if you have a weight of 200 and another of 100 you end up with 300 and a random number between 0 and 300 picked so you are twice as likely to get one < 200 than you are to get one between >= 200 <= 300. So kinda like a weighted coin flip it wont be super predictable but will give you the weighting. At least this is how I read it

file.NewZone(name, ""),
map[dns.RR]rrData{},
}

Copy link
Contributor

Choose a reason for hiding this comment

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

might be worth adding a comment that highlights this gets called multiple times otherwise at first reading seems only GEO or Weighted rather than Geo and Weighted

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants