Add IPv6 support for EndpointSlice creation and port replacement#16591
Conversation
knative-prow
Bot
added
the
size/M
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16591 +/- ##
==========================================
+ Coverage 80.21% 80.26% +0.04%
==========================================
Files 217 217
Lines 13545 13552 +7
==========================================
+ Hits 10865 10877 +12
+ Misses 2316 2310 -6
- Partials 364 365 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| } | ||
|
|
||
| func addressTypeForIP(ip string) discoveryv1.AddressType { | ||
| parsed := net.ParseIP(ip) |
There was a problem hiding this comment.
Oh good suggestion, wasn't aware of that, thanks 👍 It even exists since go 1.18 🙈
Will update the PR
|
/retest |
3 similar comments
|
/retest |
|
/retest |
|
/retest |
|
/retest I mean all other e2e tests pass, very unlikely that this failure comes from my change but too lazy to investigate right now at least Edit: Oh it might need an artifact that got removed, I'll just rebase and push again. |
The autoscaler stat forwarder hardcoded AddressTypeIPv4 when creating EndpointSlices, which breaks on IPv6-only or dual-stack clusters. Additionally, useSecurePort used strings.Split on ":" to extract the host, which breaks on bracketed IPv6 addresses like [::1]:8012. Changes: - Add addressTypeForIP helper to dynamically determine AddressType - Fix useSecurePort to use net.SplitHostPort/net.JoinHostPort - Add tests for both functions with IPv4 and IPv6 cases Signed-off-by: Vincent Link <vlink@redhat.com>
|
/retest |
|
/lgtm |
|
thanks! |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dprotaso, linkvt The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
knative-prow
Bot
added
the
approved
3 participants
The autoscaler stat forwarder hardcoded AddressTypeIPv4 when creating EndpointSlices, which breaks on IPv6-only or dual-stack clusters. Additionally, useSecurePort used strings.Split on ":" to extract the host, which breaks on bracketed IPv6 addresses like [::1]:8012.
The issue in useSecurePort was spotted by AI, it does make sense to me though.
Reported by @matejvasek and verified locally in my IPv6 kind cluster, thanks again!
PR for fixing kourier will also follow.
Proposed Changes
/kind bug
Release Note