-
Notifications
You must be signed in to change notification settings - Fork 101
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
Consume coredns crd plugin with WRR functionality #1141
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,13 +11,12 @@ data: | |
errors | ||
health | ||
ready | ||
loadbalance round_robin | ||
prometheus 0.0.0.0:9153 | ||
forward . /etc/resolv.conf | ||
k8s_crd { | ||
resources DNSEndpoint | ||
filter k8gb.absa.oss/dnstype=local | ||
negttl {{ .Values.k8gb.dnsZoneNegTTL }} | ||
loadbalance weight | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. isn't it hardcoding particular method? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would be great to get confirmation from @kuritka , but if I understand correctly it does standard round-robin if no WRR in Gslb spec, see k8gb-io/coredns-crd-plugin#45 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @ytsarev, RR is only a subset of WRR. If weights are not defined , the RR is automatically applied. If weights are defined in CR, the WRR is used. If an error occurs for any reason, then the RR is used automatically. tahts what The reason why I defines this field is for extendability. I was planning that in the future there will be a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perfect, thank you! |
||
} | ||
} | ||
{{- end }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kuritka it's not immediately clear from k8gb-io/coredns-crd-plugin#45 - why do we need to remove this config line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resources DNSEndpoint
field was meaningless, we didn't work with it in any way.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kuritka cool, I was mostly wondering why it actually breaking the terratest, but cool to get rid of it any case, thanks!