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
check ip family for node port connectivity test #88417
Conversation
/assign @neolit123 |
/test pull-kubernetes-e2e-kind |
1 similar comment
/test pull-kubernetes-e2e-kind |
/uncc |
2185a07
to
e2c1520
Compare
@aramase |
e2c1520
to
c71eff3
Compare
/cc @aojea |
@@ -879,7 +890,7 @@ func (j *TestJig) checkNodePortServiceReachability(svc *v1.Service, pod *v1.Pod) | |||
if err != nil { | |||
return err | |||
} | |||
err = testReachabilityOverNodePorts(nodes, servicePort, pod) | |||
err = testReachabilityOverNodePorts(nodes, servicePort, pod, clusterIP) |
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.
just food for thought, should we pass the clusterIP or a isIPv6 bool
?
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.
we can do that, but just figured the testReachabilityOverNodePorts
should make the decision on what it checks and has to do with the clusterIP
. WDYT?
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.
it seems correct one or the other, I don't have a strong opinion about this, this looks ok
c71eff3
to
bc81e06
Compare
bc81e06
to
c811fc5
Compare
/lgtm |
/test pull-kubernetes-e2e-gce |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aramase, neolit123 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 |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
The current
testReachabilityOverNodePorts
tests against all nodeInternalIPs
. It's possible a single stack v4/v6 cluster is supported with dual stack hosts in which case the cloud providers would update theInternalIP
with both. This PR checks the ip family of theclusterIP
and checks node port connectivity on allInternalIP
that belong to the same family.Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: