Add IPv6 support to the Container Lifecycle tests #75245
Conversation
Current e2e tests for the Container Lifecycle Hooks weren't using brackets for the IPv6 URL addresses per RFC2732, thus those tests were failing. This patches add brackets to the target URL if it's an IPv6 address. Reference: kubernetes#70248
|
@aojea: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
k8s-ci-robot
added
kind/failing-test
|
Hi @aojea. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
k8s-ci-robot
added
the
needs-ok-to-test
k8s-ci-robot
added
area/test
sig/testing
|
/assign @janetkuo |
|
/assign @bowei |
|
/cc @MrHohn |
| @@ -38,7 +40,7 @@ var _ = framework.KubeDescribe("Container Lifecycle Hook", func() { | |||
| preStopWaitTimeout = 30 * time.Second | |||
| ) | |||
| Context("when create a pod with lifecycle hook", func() { | |||
| var targetIP string | |||
| var targetIP, targetURL string | |||
There was a problem hiding this comment.
I was hoping we can have a more consistent way to handle IPv6 cases like this --- just a bit worried to see subtle logic scattered around.
Though I guess this is okay for now?
There was a problem hiding this comment.
@MrHohn I can add a function to build correct IPv6 urls so it can be reused
// FormatLiteralIPURL Returns a valid URL for literal IP addresses
// https://tools.ietf.org/html/rfc2732
func FormatLiteralIPURL (ip string) string {
if strings.Contains(ip, ":") {
return fmt.Sprintf("[%s]", ip)
}
return ip
}
just let me know what will be the best place to add it
There was a problem hiding this comment.
Looks good, probably add to https://github.com/kubernetes/kubernetes/blob/master/test/e2e/framework/networking_utils.go for now. We can find a more common place outside of the test folder later if needed.
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aojea, MrHohn 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
|
/ok-to-test |
k8s-ci-robot
added
release-note-none
k8s-ci-robot
added
ok-to-test
|
/retest Review the full test history for this PR. Silence the bot with an |
What type of PR is this?
What this PR does / why we need it:
Current e2e tests for the Container Lifecycle Hooks weren't
using brackets for the IPv6 URL addresses per RFC2732, thus those
tests were failing.
This patch add brackets to the target URL if it's an IPv6 address
Which issue(s) this PR fixes:
Fixes Partially #70248
Special notes for your reviewer:
Does this PR introduce a user-facing change?: