Add checking all IPs in Service reachibility verification #2188
Conversation
k8s-ci-robot
added
kind/feature
This is adding support for Dual Stack Kubernetes services
panslava
force-pushed
the
dual-stack-reachable
branch
from
d561071
to
27dee97
Compare
k8s-ci-robot
added
size/S
|
/test pull-perf-tests-clusterloader2-kubemark |
|
/retest |
|
/test pull-perf-tests-clusterloader2-kubemark |
| address := net.JoinHostPort(ip, fmt.Sprint(port)) | ||
| command := fmt.Sprintf("curl %s", address) | ||
| _, err = execservice.RunCommand(pod, command) | ||
| if err != nil { |
There was a problem hiding this comment.
Consider using ErrorList for collecting errors from attempted connections to all the IP addresses rather than immediately breaking from the loop on receiving the first non-nil error here.
There was a problem hiding this comment.
I agree that would be useful, if we wanted to return errors, or log them
But here we just want to check if both IPs are reachable, what is the benefit of checking second one, if we know, that first one already failed?
There was a problem hiding this comment.
Ah, I didn't read the rest of this measurement's logic. I thought we're plugging this error through somewhere higher up the call stack and wanted to know if, for instance, IPv6 network programming failed but IPv4 did not 🤷.
In that case I'm okay with leaving it as it is.
|
/retest |
|
/retest |
k8s-ci-robot
added
the
lgtm
|
/approve |
|
/assign @marseel |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: marseel, mborsz, panslava, tosi3k The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
This is adding support for Dual Stack Kubernetes services
What type of PR is this?
/kind feature
What this PR does / why we need it:
When checking if service is reachable we were checking only first Service IP
This PR adds checking all IPs, this adds support for DualStack services, which can have 2 IPs