Skip to content

Commit 2bacbf4

Browse files
committed
HBASE-30355 setupMiniKdc port-conflict retry never triggers because Kerby throws KrbException, not BindException
The retry-on-port-conflict block in setupMiniKdc only catches java.net.BindException, but the Kerby-backed MiniKdc wraps the bind failure in a KrbException with no BindException in its cause chain - the conflict is only visible via the "Address already in use" message. The retry was therefore dead code and the first port collision failed the test. Recognise the bind conflict regardless of wrapper type: catch Exception, and before retrying, test a predicate that walks the whole cause chain and matches the "Address already in use" message; rethrow anything else so a genuine KDC misconfig is not masked. We avoid catch (BindException | KrbException) because the message (not the type) is the discriminator and to avoid importing kerby types here (see HBASE-29117). Applied to both TestLogLevel.setupMiniKdc (hbase-http) and HBaseTestingUtil.setupMiniKdc (hbase-server). Tests: - testKdcBindConflictSurfacesAsKrbException: deterministic reproduction occupying the KDC port on TCP+UDP; asserts the failure is a KrbException the predicate recognises. - testIsBindExceptionRecognizesKerbyWrappedBindFailure: unit test for the predicate (wrapped BindException, message-only, and negative case).
1 parent c5bd5c5 commit 2bacbf4

2 files changed

Lines changed: 109 additions & 4 deletions

File tree

hbase-http/src/test/java/org/apache/hadoop/hbase/http/log/TestLogLevel.java

Lines changed: 82 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,16 @@
2121
import static org.junit.jupiter.api.Assertions.assertFalse;
2222
import static org.junit.jupiter.api.Assertions.assertNotEquals;
2323
import static org.junit.jupiter.api.Assertions.assertSame;
24+
import static org.junit.jupiter.api.Assertions.assertThrows;
2425
import static org.junit.jupiter.api.Assertions.assertTrue;
2526
import static org.junit.jupiter.api.Assertions.fail;
2627

2728
import java.io.File;
2829
import java.io.IOException;
2930
import java.net.BindException;
31+
import java.net.DatagramSocket;
32+
import java.net.InetAddress;
33+
import java.net.ServerSocket;
3034
import java.net.SocketException;
3135
import java.net.URI;
3236
import java.security.PrivilegedExceptionAction;
@@ -57,6 +61,7 @@
5761
import org.apache.hadoop.security.ssl.SSLFactory;
5862
import org.apache.hadoop.test.GenericTestUtils;
5963
import org.apache.hadoop.util.StringUtils;
64+
import org.apache.kerby.kerberos.kerb.KrbException;
6065
import org.junit.jupiter.api.AfterAll;
6166
import org.junit.jupiter.api.BeforeAll;
6267
import org.junit.jupiter.api.Tag;
@@ -153,20 +158,95 @@ static private MiniKdc setupMiniKdc() throws Exception {
153158
dir = new File(HTU.getDataTestDir("kdc").toUri().getPath());
154159
kdc = new MiniKdc(conf, dir);
155160
kdc.start();
156-
} catch (BindException e) {
161+
} catch (Exception e) {
162+
// Catch Exception, not BindException/KrbException: Kerby wraps the bind failure in a
163+
// KrbException (and the cause chain carries no BindException), so the port conflict is only
164+
// recognisable by message. We also avoid importing kerby types here (see HBASE-29117).
165+
if (!isBindException(e)) {
166+
throw e; // not a port conflict, do not mask the real failure behind a retry
167+
}
157168
FileUtils.deleteDirectory(dir); // clean directory
158169
numTries++;
159170
if (numTries == 3) {
160171
log.error("Failed setting up MiniKDC. Tried " + numTries + " times.");
161172
throw e;
162173
}
163-
log.error("BindException encountered when setting up MiniKdc. Trying again.");
174+
log.error("Bind conflict encountered when setting up MiniKdc, retrying (attempt " + numTries
175+
+ ").");
164176
bindException = true;
165177
}
166178
} while (bindException);
167179
return kdc;
168180
}
169181

182+
/**
183+
* The Kerby-backed {@link MiniKdc} wraps a failure to bind the KDC port in a
184+
* {@code org.apache.kerby...KrbException} rather than surfacing a {@link BindException} directly,
185+
* so we inspect the whole cause chain plus the message to recognise a port conflict.
186+
*/
187+
static boolean isBindException(Throwable t) {
188+
for (Throwable cause = t; cause != null; cause = cause.getCause()) {
189+
if (cause instanceof BindException) {
190+
return true;
191+
}
192+
String msg = cause.getMessage();
193+
if (msg != null && msg.contains("Address already in use")) {
194+
return true;
195+
}
196+
}
197+
return false;
198+
}
199+
200+
/**
201+
* Reproduces the port-conflict flake deterministically: occupy the KDC port on both TCP and UDP,
202+
* pin MiniKdc to it, and show the resulting failure is a Kerby {@code KrbException}, not a
203+
* {@link BindException}. This is why the {@code catch (BindException)} retry in
204+
* {@link #setupMiniKdc()} never fired for the reported failure.
205+
*/
206+
@Test
207+
public void testKdcBindConflictSurfacesAsKrbException() throws Exception {
208+
int port;
209+
try (ServerSocket probe = new ServerSocket(0, 1, InetAddress.getByName(LOCALHOST))) {
210+
port = probe.getLocalPort();
211+
}
212+
try (ServerSocket tcp = new ServerSocket(port, 1, InetAddress.getByName(LOCALHOST));
213+
DatagramSocket udp = new DatagramSocket(port, InetAddress.getByName(LOCALHOST))) {
214+
Properties conf = MiniKdc.createConf();
215+
conf.put(MiniKdc.DEBUG, true);
216+
conf.setProperty(MiniKdc.KDC_BIND_ADDRESS, LOCALHOST);
217+
conf.setProperty(MiniKdc.KDC_PORT, Integer.toString(port));
218+
File dir = new File(HTU.getDataTestDir("kdc-conflict").toUri().getPath());
219+
MiniKdc conflictingKdc = new MiniKdc(conf, dir);
220+
221+
Exception thrown = assertThrows(Exception.class, conflictingKdc::start);
222+
223+
assertFalse(thrown instanceof BindException,
224+
"Kerby wraps the bind failure in a KrbException, so it is not a BindException: " + thrown);
225+
assertTrue(isBindException(thrown),
226+
"expected a recognisable bind-conflict failure, got: " + thrown);
227+
}
228+
}
229+
230+
/**
231+
* Deterministic regression test for the retry predicate: a Kerby {@code KrbException} caused by a
232+
* port conflict must be recognised as a bind failure, whether the {@link BindException} is
233+
* preserved in the cause chain or only reflected in the message. Unrelated failures must not be.
234+
*/
235+
@Test
236+
public void testIsBindExceptionRecognizesKerbyWrappedBindFailure() {
237+
assertTrue(
238+
isBindException(
239+
new KrbException("Failed to start DefaultKrbServer. Address already in use (Bind failed)",
240+
new BindException("Address already in use (Bind failed)"))),
241+
"KrbException wrapping a BindException should be recognised");
242+
assertTrue(
243+
isBindException(
244+
new KrbException("Failed to start DefaultKrbServer. Address already in use (Bind failed)")),
245+
"KrbException whose message reports the bind conflict should be recognised");
246+
assertFalse(isBindException(new RuntimeException("boom")),
247+
"an unrelated failure must not be treated as a retryable bind conflict");
248+
}
249+
170250
static private void setupSSL(File base) throws Exception {
171251
clientConf.set(DFSConfigKeys.DFS_HTTP_POLICY_KEY, HttpConfig.Policy.HTTPS_ONLY.name());
172252
clientConf.set(DFSConfigKeys.DFS_NAMENODE_HTTPS_ADDRESS_KEY, "localhost:0");

hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtil.java

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3731,21 +3731,46 @@ public MiniKdc setupMiniKdc(File keytabFile) throws Exception {
37313731
dir = new File(getDataTestDir("kdc").toUri().getPath());
37323732
kdc = new MiniKdc(conf, dir);
37333733
kdc.start();
3734-
} catch (BindException e) {
3734+
} catch (Exception e) {
3735+
// Catch Exception, not BindException/KrbException: Kerby wraps the bind failure in a
3736+
// KrbException (and the cause chain carries no BindException), so the port conflict is only
3737+
// recognisable by message. We also avoid importing kerby types here (see HBASE-29117).
3738+
if (!isBindException(e)) {
3739+
throw e; // not a port conflict, do not mask the real failure behind a retry
3740+
}
37353741
FileUtils.deleteDirectory(dir); // clean directory
37363742
numTries++;
37373743
if (numTries == 3) {
37383744
LOG.error("Failed setting up MiniKDC. Tried " + numTries + " times.");
37393745
throw e;
37403746
}
3741-
LOG.error("BindException encountered when setting up MiniKdc. Trying again.");
3747+
LOG.error("Bind conflict encountered when setting up MiniKdc, retrying (attempt " + numTries
3748+
+ ").");
37423749
bindException = true;
37433750
}
37443751
} while (bindException);
37453752
HBaseKerberosUtils.setKeytabFileForTesting(keytabFile.getAbsolutePath());
37463753
return kdc;
37473754
}
37483755

3756+
/**
3757+
* The Kerby-backed {@link MiniKdc} wraps a failure to bind the KDC port in a
3758+
* {@code org.apache.kerby...KrbException} rather than surfacing a {@link BindException} directly,
3759+
* so we inspect the whole cause chain plus the message to recognise a port conflict.
3760+
*/
3761+
static boolean isBindException(Throwable t) {
3762+
for (Throwable cause = t; cause != null; cause = cause.getCause()) {
3763+
if (cause instanceof BindException) {
3764+
return true;
3765+
}
3766+
String msg = cause.getMessage();
3767+
if (msg != null && msg.contains("Address already in use")) {
3768+
return true;
3769+
}
3770+
}
3771+
return false;
3772+
}
3773+
37493774
public int getNumHFiles(final TableName tableName, final byte[] family) {
37503775
int numHFiles = 0;
37513776
for (RegionServerThread regionServerThread : getMiniHBaseCluster().getRegionServerThreads()) {

0 commit comments

Comments
 (0)