return PTR record if pod hostname is not set #7181
Conversation
Signed-off-by: Jack Francis <jackfrancis@gmail.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #7181 +/- ##
==========================================
+ Coverage 55.70% 57.79% +2.09%
==========================================
Files 224 270 +46
Lines 10016 17383 +7367
==========================================
+ Hits 5579 10046 +4467
- Misses 3978 6727 +2749
- Partials 459 610 +151 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| if addr.Hostname != "" { | ||
| // If the endpoint's Hostname is set, compose a compliant PTR record composed from that hostname | ||
| // Kubernetes more or less keeps this to one canonical service/endpoint per IP, but in the odd event there | ||
| // are multiple endpoints for the same IP with hostname set, return them all rather than selecting one |
There was a problem hiding this comment.
I don't see a test for this behavior. The two-service selection test returns a single PTR.
Also why should we only return multiple in the case where hostname is set? I am confused as to what we actually want right now.
There was a problem hiding this comment.
This is not (meant to be) net new behavior, it is a reformulation of the existing behavior. I'm relying on the existing UT to validate that this change doesn't yield any new behaviors.
Also, to be clear, this change returns multiple records in both scenarios (hostname or no hostname). It just doesn't return a mixed set (multiple records, some of which have hostnames, some which do not). That preserves the post-6898 behaviors.
Taking a step back I agree that returning multiple records is weird, but that is what the current code at HEAD currently does. Here's a UT case to prove it (to be clear I don't think this is good! just trying not to break existing behaviors):
There was a problem hiding this comment.
Added more UT cases to this PR as well to show more permutations
1. Why is this pull request needed and what does it do?
This PR restores the previous behavior of returning a well-formed PTR record for DNS records that correlate with pods without a hostname. It is compatible with the changes in #6898 whose intention (among other things) was to improve the deterministic response of PTR records for pod IPs sharing a common hostname.
2. Which issues (if any) are related?
Fixes #7177
3. Which documentation changes (if any) need to be made?
4. Does this introduce a backward incompatible change or deprecation?