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

Use loopback network interface for testcases. #1097

Closed
wants to merge 1 commit into from

Conversation

reddycharan
Copy link
Contributor

@reddycharan reddycharan commented Feb 1, 2018

Descriptions of the changes in this PR:

  • tests would be more reliable irrespective of the environment (and availability of network connection), if the loopback address is used in testsuites.
  • unless loopback address is set explicitly, in my env (while on company's VPN),
    UpdateCookieCmdTest is failing most of the times.
  • currently allowLoopback is set to true in BookKeeperClusterTestCase,
    but it doesn't make Bookies to use loopback interface address.
  • loopback network interface should be set explicitly as
    listening interface to use loopback address.

Note:

  1. will create Issue for this depending on the PR feedback.
  2. in other places (where setAllowLoopback is set) also loopback interface needs to be set

- tests would be more reliable irrespective of the environment,
if the loopback address is used in testsuites.
- unless loopback address is set explicitly, in my env (while on company's VPN),
UpdateCookieCmdTest is failing most of the times.
- currently allowLoopback is set to true in BookKeeperClusterTestCase,
it doesn't make Bookies to use loopback interface address.
- loopback network interface should be set explicitly as
listening interface to use loopback address.
@athanatos
Copy link

lgtm

Copy link
Member

@jiazhai jiazhai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1. lgtm.
Would you please help check whether the failed test cases is related to this ?

@jiazhai
Copy link
Member

jiazhai commented Feb 2, 2018

retest this please

@reddycharan
Copy link
Contributor Author

reddycharan commented Feb 3, 2018

I ran locally AuditorLedgerCheckerTest and BookieClientTest multiple times and they are running fine. Not sure why tests failed here. Also different tests failed in Java8 and Java9 build.

@reddycharan
Copy link
Contributor Author

@sijie I made changes to the initial commit and replaced the initial commit. Can you sign-off on this. Thanks.

@sijie
Copy link
Member

sijie commented Feb 3, 2018

@reddycharan I don't think the failures are related.

@sijie sijie added this to the 4.7.0 milestone Feb 3, 2018
@sijie sijie closed this in ee2f419 Feb 3, 2018
sijie added a commit to sijie/bookkeeper that referenced this pull request Feb 19, 2018
*Problem*

Issue apache#1097 introduced using `loopback` nic as the bookie server identifier. However a few test cases construct
the bookie socket address using `InetAddress.getLocalHost()` and the bookie port. This bookie socket address can
be different from the actual socket address that a bookie is advertising to zookeeper. It will cause test cases
fail with some network settings.

*Solution*

This change use `BookieServer#getLocalAddress()` to retrieve the actual bookie socket address. so it would respect
to the server configuration and use the right socket address in the test.
sijie added a commit that referenced this pull request Feb 20, 2018
Descriptions of the changes in this PR:

*Problem*

Issue #1097 introduced using `loopback` nic as the bookie server identifier. However a few test cases construct the bookie socket address using `InetAddress.getLocalHost()` and the bookie port. This bookie socket address can be different from the actual socket address that a bookie is advertising to zookeeper. It will cause test cases fail with certain network settings.

*Solution*

This change use `BookieServer#getLocalAddress()` to retrieve the actual bookie socket address. so it would respect to the server configuration and use the right socket address in the test.

Author: Sijie Guo <sijie@apache.org>

Reviewers: Charan Reddy Guttapalem <reddycharan18@gmail.com>, Enrico Olivelli <eolivelli@gmail.com>

This closes #1172 from sijie/use_bookie_server_get_local_address
@sijie sijie mentioned this pull request Feb 21, 2018
sijie added a commit that referenced this pull request Feb 21, 2018
Descriptions of the changes in this PR:

*Problem*

in GSSAPIBookKeeperTest, `InetAddress.getLocalHost().getHostName()` was used for adding principle to kdc keytab. However in the tests, it uses advertisedAddress (introduced by #1097).

So it causes authentication failed because server can't be found.

*Solution*

Change to use `Bookie. getBookieAddress(..)` to get the real hostname used by bookie for adding keytab entries.

Author: Sijie Guo <sijie@apache.org>

Reviewers: Charan Reddy Guttapalem <reddycharan18@gmail.com>, Enrico Olivelli <eolivelli@gmail.com>

This closes #1191 from sijie/fix_gssapibookkeeper_test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants