Skip to content
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

Split TestLoopbackHostPort into 2 tests #76966

Merged
merged 1 commit into from May 7, 2019

Conversation

@figo
Copy link
Member

commented Apr 23, 2019

firstly, split into two tests: TestLoopbackHostPortIPv4 and TestLoopbackHostPortIPv6.
then improve error handling, going to fail with explicit error message when run host
that does not support ipv6 or ipv4

What type of PR is this?

/kind bug
/kind failing-test
/kind flake

What this PR does / why we need it:
fixing a failing test on host that does not support ipv6 loopback.

Which issue(s) this PR fixes:

Fixes: #76393

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

N/A

cc @freehan @spiffxp @dims @timothysc @lavalamp

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Apr 23, 2019

Keywords which can automatically close issues and at(@) mentions are not allowed in commit messages.

The list of commits with invalid commit messages:

  • d59fc70 skip ipv6 part of test if host does not support

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. I understand the commands that are listed here.

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Apr 23, 2019

Keywords which can automatically close issues and at(@) mentions are not allowed in commit messages.

The list of commits with invalid commit messages:

  • 0ff27c2 Skip ipv6 part of TestLoopbackHostPort when needed

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. I understand the commands that are listed here.

@figo figo force-pushed the figo:master branch from 0ff27c2 to 06ac49c Apr 23, 2019

@figo figo changed the title skip ipv6 part of test if host does not support Skip ipv6 part of TestLoopbackHostPort on host the does not support ipv6 loopback Apr 23, 2019

@figo

This comment has been minimized.

Copy link
Member Author

commented Apr 23, 2019

/test pull-kubernetes-e2e-gce-100-performance

@figo figo force-pushed the figo:master branch from cb61208 to bc92280 Apr 24, 2019

@figo

This comment has been minimized.

Copy link
Member Author

commented Apr 24, 2019

/assign @deads2k @liggitt

@figo figo force-pushed the figo:master branch from bc92280 to 6db6cbb Apr 26, 2019

@figo figo changed the title Skip ipv6 part of TestLoopbackHostPort on host the does not support ipv6 loopback Split TestLoopbackHostPort into 2 tests Apr 26, 2019

@andrewsykim
Copy link
Member

left a comment

/kind failing-test
/priority important-longterm

@figo figo force-pushed the figo:master branch 2 times, most recently from 10ca823 to 0effe80 Apr 26, 2019

@figo

This comment has been minimized.

Copy link
Member Author

commented May 2, 2019

@deads2k @lavalamp could you take another look, thx

if port != "443" {
t.Fatalf("expected 443 as port, got %q", port)
}
}

func isIPv6LoopbackSupported() (bool, bool, error) {

This comment has been minimized.

Copy link
@lavalamp

lavalamp May 6, 2019

Member

If you change it to be like this, it's self-documenting:

func supportedLoopbacks() (ipv4 bool, ipv6 bool, err error) {

This comment has been minimized.

Copy link
@figo

figo May 6, 2019

Author Member

good suggestion, done, @lavalamp

This comment has been minimized.

Copy link
@lavalamp

lavalamp May 6, 2019

Member

Thanks for fixing--as evidence that it was a good change, clearly I had misunderstood what the bools meant, haha!

@lavalamp

This comment has been minimized.

Copy link
Member

commented May 6, 2019

I left one tiny nit, sorry!

Also sorry for slow response--I spent most of last week sick.

@figo figo force-pushed the figo:master branch from 0effe80 to c089861 May 6, 2019

@lavalamp

This comment has been minimized.

Copy link
Member

commented May 6, 2019

/approve

Can you squash please?

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented May 6, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: figo, lavalamp

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Split TestLoopbackHostPort into 2 tests
firstly, split into two tests: TestLoopbackHostPortIPv4 and  TestLoopbackHostPortIPv6.
then improve error handling, going to fail with explicit error message when run host
that does not support ipv6 or ipv4

@figo figo force-pushed the figo:master branch from c089861 to 6b73b50 May 6, 2019

@figo

This comment has been minimized.

Copy link
Member Author

commented May 6, 2019

/approve

Can you squash please?

@lavalamp done, please add lgtm if you are fine with the change, thx

@andrewsykim

This comment has been minimized.

Copy link
Member

commented May 6, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label May 6, 2019

@k8s-ci-robot k8s-ci-robot merged commit 1b4b1d1 into kubernetes:master May 7, 2019

20 checks passed

cla/linuxfoundation figo authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-conformance-image-test Skipped.
pull-kubernetes-cross Skipped.
pull-kubernetes-dependencies Job succeeded.
Details
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-csi-serial Skipped.
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gce-storage-slow Skipped.
pull-kubernetes-godeps Skipped.
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped.
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
pull-publishing-bot-validate Skipped.
tide In merge pool.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.