ZOOKEEPER-4303: Allow configuring client port to 0 #1868
Conversation
|
I'm not able to reproduce the failure seen by the GitHub actions, can somebody help me figure out if this is caused by my changes or an existing issue? |
|
Force pushed rebase to development branch. |
| @@ -427,7 +427,7 @@ public void parseProperties(Properties zkProp) throws IOException, ConfigExcepti | |||
| dataLogDir = dataDir; | |||
| } | |||
|
|
|||
| if (clientPort == 0) { | |||
| if (clientPort == null) { | |||
There was a problem hiding this comment.
This is a behavior change but I think it make sense. Prior to this, "clientPortAddress" and "clientPort": 0 is not allowed. It is better to allow bind to 0 with specified local address.
There was a problem hiding this comment.
The implicit logic here is that: use serverId to lookup if not address configured. Prior to this, 0 was used as "not configured", so explicit "127.0.0.1:0" is denied.
| assertThat(zkServer.getConnectionString(), not(endsWith(":0"))); | ||
| assertTrue(ClientBase.waitForServerUp(zkServer.getConnectionString(), 60000)); | ||
| } | ||
| } |
There was a problem hiding this comment.
Would you mind add one more case to assert explicit "127.0.0.1:0" ? As I said above, I think it is a noticeable change.
| return prettifyConnectionString(config.getSecureClientPortAddress(), boundSecureClientPort); | ||
| } | ||
|
|
||
| private String prettifyConnectionString(InetSocketAddress confAddress, int boundPort) { |
|
I have restarted CI |
|
@maoling @symat Could you please take a look at this ? It is valuable in testing/embedded environments to avoid port conflict. It is ZooKeeper's CURATOR-535. |
|
@eolivelli @symat Shall we merge this ? |
|
sorry for the delay, I kind of forgot about this one, I can merge it :( |
|
I think that we can merge it. I believe it is useless now that we have GH actions, I already started a discussion on dev@ |
|
yes, I agree to eliminate some of these CI jobs. I'll merge this PR now. |
|
merge is done, thank you @madrob for your contribution! |
https://issues.apache.org/jira/browse/ZOOKEEPER-4303 Allows specifying an explicit port 0 for the client port or secure client port, which will default to operating system behavior of finding an unbound port. Modified ZKServerEmbedded to report the actual port instead of the configured port. Author: Mike Drob <mdrob@apple.com> Reviewers: Kezhu Wang <kezhuw@gmail.com>, Enrico Olivelli <eolivelli@apache.org>, Mate Szalay-Beko <symat@apache.org> Closes apache#1868 from madrob/zookeeper-4303
https://issues.apache.org/jira/browse/ZOOKEEPER-4303 Allows specifying an explicit port 0 for the client port or secure client port, which will default to operating system behavior of finding an unbound port. Modified ZKServerEmbedded to report the actual port instead of the configured port. Author: Mike Drob <mdrob@apple.com> Reviewers: Kezhu Wang <kezhuw@gmail.com>, Enrico Olivelli <eolivelli@apache.org>, Mate Szalay-Beko <symat@apache.org> Closes apache#1868 from madrob/zookeeper-4303 Co-authored-by: Mike Drob <mdrob@apple.com>
|
@eolivelli @symat Would you mind merging this to 3.7 and 3.8 ? apache/curator#421 needs this. |
https://issues.apache.org/jira/browse/ZOOKEEPER-4303
Allows specifying an explicit port 0 for the client port or secure client port, which will default to operating system behavior of finding an unbound port. Modified ZKServerEmbedded to report the actual port instead of the configured port.