diff --git a/src/main/java/org/metricshub/ipmi/client/IpmiClient.java b/src/main/java/org/metricshub/ipmi/client/IpmiClient.java index 9c56610..7b4f491 100644 --- a/src/main/java/org/metricshub/ipmi/client/IpmiClient.java +++ b/src/main/java/org/metricshub/ipmi/client/IpmiClient.java @@ -55,9 +55,7 @@ public static GetChassisStatusResponseData getChassisStatus(final IpmiClientConf throws InterruptedException, ExecutionException, TimeoutException { - try (GetChassisStatusRunner runner = new GetChassisStatusRunner(ipmiConfiguration)) { - return execute(runner, ipmiConfiguration.getTimeout() * 1000); - } + return execute(new GetChassisStatusRunner(ipmiConfiguration), ipmiConfiguration.getTimeout() * 1000); } /** @@ -73,9 +71,7 @@ public static List getSensors(final IpmiClientConfiguration ipmiConfigur throws InterruptedException, ExecutionException, TimeoutException { - try (GetSensorsRunner runner = new GetSensorsRunner(ipmiConfiguration)) { - return execute(runner, ipmiConfiguration.getTimeout() * 1000); - } + return execute(new GetSensorsRunner(ipmiConfiguration), ipmiConfiguration.getTimeout() * 1000); } /** @@ -91,9 +87,7 @@ public static List getFrus(final IpmiClientConfiguration ipmiConfiguration) throws InterruptedException, ExecutionException, TimeoutException { - try (GetFrusRunner runner = new GetFrusRunner(ipmiConfiguration)) { - return execute(runner, ipmiConfiguration.getTimeout() * 1000); - } + return execute(new GetFrusRunner(ipmiConfiguration), ipmiConfiguration.getTimeout() * 1000); } /** diff --git a/src/main/java/org/metricshub/ipmi/client/Utils.java b/src/main/java/org/metricshub/ipmi/client/Utils.java index 7bb1982..9cf6d7b 100644 --- a/src/main/java/org/metricshub/ipmi/client/Utils.java +++ b/src/main/java/org/metricshub/ipmi/client/Utils.java @@ -31,13 +31,22 @@ import java.util.concurrent.TimeoutException; import org.metricshub.ipmi.client.runner.AbstractIpmiRunner; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public final class Utils { + private static final Logger LOGGER = LoggerFactory.getLogger(Utils.class); + private Utils() {} public static final String EMPTY = ""; + /** + * How long a call that hit its deadline waits for its worker to close the session and the socket. + */ + private static final long CLEANUP_GRACE_MS = 1000; + /** * @param value The value to check * @return whether the value is null, empty or contains only blank chars @@ -87,19 +96,50 @@ public static T execute(final AbstractIpmiRunner callable, long timeout) ExecutionException, TimeoutException { - final ExecutorService executorService = Executors.newSingleThreadExecutor(); - final Future future = executorService.submit(callable); + final ExecutorService executorService = Executors.newSingleThreadExecutor(Utils::newWorkerThread); + + // The worker owns the connector: it also closes it, so the cleanup is covered by the deadline and never + // runs on the calling thread while the worker is still using the connection + final Future future = executorService.submit(() -> { + try (AbstractIpmiRunner runner = callable) { + return runner.call(); + } + }); try { return future.get(timeout, TimeUnit.MILLISECONDS); } catch (InterruptedException e) { + stopWorker(future, executorService); Thread.currentThread().interrupt(); throw e; } catch (TimeoutException e) { - future.cancel(true); + stopWorker(future, executorService); throw e; } finally { executorService.shutdownNow(); } } + + /** + * Stops the worker at its current wait and gives it a moment to close the session and release the port. + */ + private static void stopWorker(Future future, ExecutorService executorService) { + future.cancel(true); + executorService.shutdownNow(); + try { + if (!executorService.awaitTermination(CLEANUP_GRACE_MS, TimeUnit.MILLISECONDS)) { + // A call that cannot be interrupted (name resolution, a blocking send): the worker closes the + // connection and releases the port by itself when that call returns + LOGGER.warn("The IPMI worker is still busy after the call was abandoned; the port is released when it returns"); + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + + private static Thread newWorkerThread(Runnable runnable) { + Thread thread = new Thread(runnable, "ipmi-client"); + thread.setDaemon(true); + return thread; + } } diff --git a/src/main/java/org/metricshub/ipmi/client/runner/AbstractIpmiRunner.java b/src/main/java/org/metricshub/ipmi/client/runner/AbstractIpmiRunner.java index 9f2802b..4635b50 100644 --- a/src/main/java/org/metricshub/ipmi/client/runner/AbstractIpmiRunner.java +++ b/src/main/java/org/metricshub/ipmi/client/runner/AbstractIpmiRunner.java @@ -243,6 +243,7 @@ public void close() { // Close connection manager and release the listener port. connector.tearDown(); + connector = null; } /** diff --git a/src/main/java/org/metricshub/ipmi/core/api/async/IpmiAsyncConnector.java b/src/main/java/org/metricshub/ipmi/core/api/async/IpmiAsyncConnector.java index d221868..3e9c037 100644 --- a/src/main/java/org/metricshub/ipmi/core/api/async/IpmiAsyncConnector.java +++ b/src/main/java/org/metricshub/ipmi/core/api/async/IpmiAsyncConnector.java @@ -30,6 +30,7 @@ import org.metricshub.ipmi.core.coding.commands.ResponseData; import org.metricshub.ipmi.core.coding.commands.session.GetChannelAuthenticationCapabilitiesResponseData; import org.metricshub.ipmi.core.coding.payload.IpmiPayload; +import org.metricshub.ipmi.core.coding.payload.lan.IPMIException; import org.metricshub.ipmi.core.coding.protocol.PayloadType; import org.metricshub.ipmi.core.coding.security.CipherSuite; import org.metricshub.ipmi.core.common.PropertiesManager; @@ -226,13 +227,11 @@ public List getAvailableCipherSuites( ++tries; result = connectionManager .getAvailableCipherSuites(connectionHandle.getHandle()); - } catch (InterruptedException e) { - throw e; } catch (Exception e) { - logger.warn(FAILED_TO_RECEIVE_ANSWER_CAUSE_MESSAGE, e); - if (tries > retries) { + if (tries > retries || !isRetriable(e)) { throw e; } + logger.warn(FAILED_TO_RECEIVE_ANSWER_CAUSE_MESSAGE, e); } } return result; @@ -272,13 +271,11 @@ public GetChannelAuthenticationCapabilitiesResponseData getChannelAuthentication requestedPrivilegeLevel); connectionHandle.setCipherSuite(cipherSuite); connectionHandle.setPrivilegeLevel(requestedPrivilegeLevel); - } catch (InterruptedException e) { - throw e; } catch (Exception e) { - logger.warn(FAILED_TO_RECEIVE_ANSWER_CAUSE_MESSAGE, e); - if (tries > retries) { + if (tries > retries || !isRetriable(e)) { throw e; } + logger.warn(FAILED_TO_RECEIVE_ANSWER_CAUSE_MESSAGE, e); } } return result; @@ -331,13 +328,11 @@ public Session openSession( session = sessionManager.registerSession(sessionId, connectionHandle); succeded = true; - } catch (InterruptedException e) { - throw e; } catch (Exception e) { - logger.warn(FAILED_TO_RECEIVE_ANSWER_CAUSE_MESSAGE, e); - if (tries > retries) { + if (tries > retries || !isRetriable(e)) { throw e; } + logger.warn(FAILED_TO_RECEIVE_ANSWER_CAUSE_MESSAGE, e); } } @@ -602,4 +597,14 @@ public int getTimeout(ConnectionHandle handle) { return connectionManager.getConnection(handle.getHandle()).getTimeout(); } + /** + * Tells whether a failed handshake step is worth sending again: when no reply came, or when the BMC answered + * with a transient completion code. Any other answer (wrong credentials, unknown user, refused cipher suite...) + * would be the same the next time, and resending the credentials trips account lockouts. + */ + private static boolean isRetriable(Exception e) { + return e instanceof ConnectionException + || (e instanceof IPMIException && ((IPMIException) e).getCompletionCode().isTransient()); + } + } diff --git a/src/main/java/org/metricshub/ipmi/core/api/sync/IpmiConnector.java b/src/main/java/org/metricshub/ipmi/core/api/sync/IpmiConnector.java index 96f5703..135f0e6 100644 --- a/src/main/java/org/metricshub/ipmi/core/api/sync/IpmiConnector.java +++ b/src/main/java/org/metricshub/ipmi/core/api/sync/IpmiConnector.java @@ -29,7 +29,6 @@ import org.metricshub.ipmi.core.coding.commands.PrivilegeLevel; import org.metricshub.ipmi.core.coding.commands.ResponseData; import org.metricshub.ipmi.core.coding.commands.session.GetChannelAuthenticationCapabilitiesResponseData; -import org.metricshub.ipmi.core.coding.payload.CompletionCode; import org.metricshub.ipmi.core.coding.payload.lan.IPMIException; import org.metricshub.ipmi.core.coding.protocol.PayloadType; import org.metricshub.ipmi.core.coding.security.CipherSuite; @@ -449,11 +448,7 @@ private void handleRetriesWhenException(int tries, Exception e) throws Exception } private void handleErrorResponse(int tries, IPMIException e) throws Exception { - if (e.getCompletionCode() == CompletionCode.InitializationInProgress - || e.getCompletionCode() == CompletionCode.InsufficientResources - || e.getCompletionCode() == CompletionCode.NodeBusy - || e.getCompletionCode() == CompletionCode.Timeout) { - + if (e.getCompletionCode().isTransient()) { handleRetriesWhenException(tries, e); } else { throw e; diff --git a/src/main/java/org/metricshub/ipmi/core/coding/payload/CompletionCode.java b/src/main/java/org/metricshub/ipmi/core/coding/payload/CompletionCode.java index bec12d4..78a7606 100644 --- a/src/main/java/org/metricshub/ipmi/core/coding/payload/CompletionCode.java +++ b/src/main/java/org/metricshub/ipmi/core/coding/payload/CompletionCode.java @@ -276,6 +276,16 @@ public int getCode() { return code; } + /** + * Tells whether the BMC may answer the same request successfully a little later. + * + * @return true for the completion codes that only report a passing condition of the BMC (busy, out of resources, + * initializing, internal timeout), false for every other code + */ + public boolean isTransient() { + return this == InitializationInProgress || this == InsufficientResources || this == NodeBusy || this == Timeout; + } + public static CompletionCode parseInt(int value) { switch (value) { case OK: diff --git a/src/main/java/org/metricshub/ipmi/core/sm/states/AuthcapWaiting.java b/src/main/java/org/metricshub/ipmi/core/sm/states/AuthcapWaiting.java index e7b21a6..82f0529 100644 --- a/src/main/java/org/metricshub/ipmi/core/sm/states/AuthcapWaiting.java +++ b/src/main/java/org/metricshub/ipmi/core/sm/states/AuthcapWaiting.java @@ -98,6 +98,7 @@ public void doAction(StateMachine stateMachine, RmcpMessage message) { .getResponseData(ipmiMessage))); } } catch (Exception e) { + stateMachine.setCurrent(new Ciphers()); stateMachine.doExternalAction(new ErrorAction(e)); } } diff --git a/src/main/java/org/metricshub/ipmi/core/sm/states/CiphersWaiting.java b/src/main/java/org/metricshub/ipmi/core/sm/states/CiphersWaiting.java index 7b62c17..1bbc53b 100644 --- a/src/main/java/org/metricshub/ipmi/core/sm/states/CiphersWaiting.java +++ b/src/main/java/org/metricshub/ipmi/core/sm/states/CiphersWaiting.java @@ -91,6 +91,7 @@ public void doTransition( 0)); ++index; } catch (Exception e) { + stateMachine.setCurrent(new Uninitialized()); stateMachine.doExternalAction(new ErrorAction(e)); } } else if (machineEvent instanceof DefaultAck) { @@ -135,6 +136,7 @@ public void doAction(StateMachine stateMachine, RmcpMessage message) { .getResponseData(ipmiMessage))); } } catch (Exception e) { + stateMachine.setCurrent(new Uninitialized()); stateMachine.doExternalAction(new ErrorAction(e)); } } diff --git a/src/main/java/org/metricshub/ipmi/core/sm/states/OpenSessionWaiting.java b/src/main/java/org/metricshub/ipmi/core/sm/states/OpenSessionWaiting.java index 1fe6763..f41f1d0 100644 --- a/src/main/java/org/metricshub/ipmi/core/sm/states/OpenSessionWaiting.java +++ b/src/main/java/org/metricshub/ipmi/core/sm/states/OpenSessionWaiting.java @@ -97,6 +97,7 @@ public void doAction(StateMachine stateMachine, RmcpMessage message) { .getResponseData(ipmiMessage))); } } catch (Exception e) { + stateMachine.setCurrent(new Authcap()); stateMachine.doExternalAction(new ErrorAction(e)); } } diff --git a/src/main/java/org/metricshub/ipmi/core/sm/states/Rakp1Waiting.java b/src/main/java/org/metricshub/ipmi/core/sm/states/Rakp1Waiting.java index cedd152..860e333 100644 --- a/src/main/java/org/metricshub/ipmi/core/sm/states/Rakp1Waiting.java +++ b/src/main/java/org/metricshub/ipmi/core/sm/states/Rakp1Waiting.java @@ -107,6 +107,7 @@ public void doAction(StateMachine stateMachine, RmcpMessage message) { .getResponseData(ipmiMessage))); } } catch (Exception e) { + stateMachine.setCurrent(new Authcap()); stateMachine.doExternalAction(new ErrorAction(e)); } } diff --git a/src/main/java/org/metricshub/ipmi/core/sm/states/Rakp3Waiting.java b/src/main/java/org/metricshub/ipmi/core/sm/states/Rakp3Waiting.java index c5f2c8e..84f4586 100644 --- a/src/main/java/org/metricshub/ipmi/core/sm/states/Rakp3Waiting.java +++ b/src/main/java/org/metricshub/ipmi/core/sm/states/Rakp3Waiting.java @@ -121,6 +121,7 @@ public void doAction(StateMachine stateMachine, RmcpMessage message) { .getResponseData(ipmiMessage))); } } catch (Exception e) { + stateMachine.setCurrent(new Authcap()); stateMachine.doExternalAction(new ErrorAction(e)); } } diff --git a/src/site/markdown/timeouts-and-errors.md b/src/site/markdown/timeouts-and-errors.md index e60aeec..31c1f83 100644 --- a/src/site/markdown/timeouts-and-errors.md +++ b/src/site/markdown/timeouts-and-errors.md @@ -21,8 +21,11 @@ the session — in a worker thread, and waits for it at most `timeout` seconds. expires, the worker is interrupted and the method throws `java.util.concurrent.TimeoutException`, with nothing collected: there are no partial results. -The interrupted worker stops at its current wait, and the library's receiving and timer threads -are daemon threads: they never keep the JVM alive. +The interrupted worker stops at its current wait, closes the session and releases the UDP port, +and the method waits up to one second for that cleanup before throwing. A worker stuck in a call +that cannot be interrupted (name resolution, for example) closes the connection when that call +returns; a `WARN` says so. The library's receiving and timer threads are daemon threads: they +never keep the JVM alive. ## Per-message timeout and retries @@ -91,7 +94,8 @@ Common causes wrapped in the `ExecutionException`: | Cause | Meaning | | --- | --- | -| `ConnectionException: Illegal connection state: Rakp1Waiting` | The RAKP handshake failed: wrong user name or password, account not allowed over LAN or at the User level. The `ERROR` log shows the actual reason (`Authentication check failed`, ...), see [#109](https://github.com/metricshub/ipmi-java/issues/109). | +| `IllegalArgumentException: Authentication check failed` | The RAKP handshake failed: the BMC's proof does not match the password or the [BMC key](configuration.html#bmc-key). The credentials are sent once. | +| `IPMIException: Unauthorized name.`, `Invalid role.`, ... | The BMC refused the session: unknown user, user not allowed over the LAN channel or at the User level, cipher suite refused ([Troubleshooting](troubleshooting.html#the-login-fails)). | | `ConnectionException: Command timed out` / `Message timed out` | No reply after all the [tries](#per-message-timeout-and-retries) of a message: `Command timed out` during the session handshake, the usual symptom of a wrong host, a closed UDP port or IPMI over LAN disabled; `Message timed out` in the session. | | `IPMIException` | The BMC answered with an error completion code. `getCompletionCode()` returns it, for example `InsufficientPrivilege` (`0xD4`). | | `IllegalArgumentException: ... is not yet implemented.` | The chosen cipher suite uses an algorithm the client does not implement (xRC4, MD5-128). See [cipher suites](preparing-the-bmc.html#cipher-suites). | diff --git a/src/site/markdown/troubleshooting.md b/src/site/markdown/troubleshooting.md index ef31c1c..012b806 100644 --- a/src/site/markdown/troubleshooting.md +++ b/src/site/markdown/troubleshooting.md @@ -1,5 +1,5 @@ -keywords: troubleshooting, timeout, illegal connection state, rakp1waiting, authentication check failed, insufficient privilege, 0xd4, ipmitool, ipmiutil, debug -description: Diagnose the usual failures of the IPMI Java Client — timeouts, authentication errors, insufficient privilege, missing sensors or FRUs, a JVM that does not exit — and compare with ipmitool and ipmiutil. +keywords: troubleshooting, timeout, command timed out, login, authentication check failed, unauthorized name, insufficient privilege, 0xd4, ipmitool, ipmiutil, debug +description: Diagnose the usual failures of the IPMI Java Client — timeouts, authentication errors, insufficient privilege, missing sensors or FRUs — and compare with ipmitool and ipmiutil. # Troubleshooting @@ -37,14 +37,12 @@ The stack trace of the `Command timed out` shows the step that was waiting. A fa `getAvailableCipherSuites` means the BMC never answered the very first request: the address, the port or the firewall is wrong, or IPMI over LAN is disabled. -## `Illegal connection state: Rakp1Waiting` +## The login fails -The `ExecutionException` wraps -`ConnectionException: Illegal connection state: Rakp1Waiting`: the RAKP handshake (the login) -failed, and the session could not be opened. The actual reason is logged just before, at the -`ERROR` level ([#109](https://github.com/metricshub/ipmi-java/issues/109)): +The RAKP handshake (the login) failed and the session could not be opened. The credentials are +sent once; the `ExecutionException` wraps the reason: -| Logged | Cause | +| Cause | Meaning | | --- | --- | | `IllegalArgumentException: Authentication check failed` | The BMC's proof does not match the password: **wrong password** (or wrong [BMC key](configuration.html#bmc-key)). | | `IPMIException: Unauthorized name.` | **Unknown user**, or a user not allowed to log in over the LAN channel. | @@ -84,12 +82,6 @@ to make it use suite 3 or 17. | A sensor reads `0.0` | The BMC flags the reading as unavailable, which is not checked yet ([#110](https://github.com/metricshub/ipmi-java/issues/110)). | | Negative processor temperatures (`CPU1 DTS = -44.0`) | Not an error: Intel *Digital Thermal Sensor* readings are the margin below the maximum junction temperature. | -## The JVM does not exit - -After a `TimeoutException`, some threads of the library may still run, and they are not daemon -threads ([#79](https://github.com/metricshub/ipmi-java/issues/79)). End command-line programs and -test harnesses with `System.exit(0)`. - ## Collecting is slow * **FRUs**: each FRU is read 16 bytes at a time, one round trip per chunk: a few seconds per FRU diff --git a/src/site/markdown/upgrading.md b/src/site/markdown/upgrading.md index 576c3b4..f2a6919 100644 --- a/src/site/markdown/upgrading.md +++ b/src/site/markdown/upgrading.md @@ -28,8 +28,15 @@ The `IpmiClient` API is unchanged, and the client is more tolerant of real-world `ExecutionException` wrapping `ConnectionException: Command timed out`, about 20 s into the call, where 1.2.02 threw `TimeoutException` at the overall timeout; * the overall timeout cancels the worker for good: the interrupted session stops at its current - wait, and the receiving and timer threads are daemon threads, so a program no longer needs - `System.exit()` to end. + wait and closes the session and the port, normally within one second of the deadline (a worker + stuck in a call that cannot be interrupted, such as name resolution, does so when that call + returns), and the receiving and timer threads are daemon threads, so a program no longer needs + `System.exit()` to end; +* a failed login fails at once with its actual cause (`IllegalArgumentException: Authentication + check failed`, `IPMIException: Unauthorized name.`), where 1.2.02 sent the credentials four + times and threw `ConnectionException: Illegal connection state: Rakp1Waiting` + ([Troubleshooting](troubleshooting.html#the-login-fails)); only a handshake step that got no + reply is sent again. Code that **extends** the library's protocol classes needs the changes below. `QueueElement` lost its `isTimedOut()`, `makeTimedOut()` and `refreshTimestamp()` methods: a timed-out message diff --git a/src/test/java/org/metricshub/ipmi/client/IpmiClientTest.java b/src/test/java/org/metricshub/ipmi/client/IpmiClientTest.java new file mode 100644 index 0000000..e4befab --- /dev/null +++ b/src/test/java/org/metricshub/ipmi/client/IpmiClientTest.java @@ -0,0 +1,77 @@ +package org.metricshub.ipmi.client; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.HashSet; +import java.util.Set; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.stream.Collectors; + +import org.junit.jupiter.api.Test; +import org.metricshub.ipmi.core.transport.FakeBmc; + +class IpmiClientTest { + + private static final long TIMEOUT_S = 1; + + /** The timers and the receiver end shortly after the socket is closed: how long to wait for them. */ + private static final long THREAD_EXIT_MS = 2000; + + private interface IpmiCall { + Object run(IpmiClientConfiguration configuration) throws Exception; + } + + @Test + void everyCallReturnsWithinTheTimeoutAndLeavesNoThreadBehind() throws Exception { + IpmiCall[] calls = { + IpmiClient::getChassisStatus, + IpmiClient::getSensors, + IpmiClient::getFrus, + IpmiClient::getChassisStatusAsStringResult, + IpmiClient::getFrusAndSensorsAsStringResult }; + + try (FakeBmc bmc = FakeBmc.silent()) { + IpmiClientConfiguration configuration = new IpmiClientConfiguration( + bmc.getAddress().getHostAddress(), + bmc.getPort(), + "user", + "password".toCharArray(), + null, + false, + TIMEOUT_S); + + for (IpmiCall call : calls) { + Set before = Thread.getAllStackTraces().keySet(); + int requestsBefore = bmc.getRequestCount(); + long start = System.nanoTime(); + + assertThrows(TimeoutException.class, () -> call.run(configuration)); + + long elapsed = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start); + assertTrue( + elapsed >= TIMEOUT_S * 1000 && elapsed < TIMEOUT_S * 1000 + 1500, + "elapsed " + elapsed + " ms"); + assertTrue(bmc.getRequestCount() > requestsBefore, "the BMC must have been contacted"); + assertEquals("", survivors(before), "library threads still alive after the call"); + } + } + } + + private static String survivors(Set before) throws InterruptedException { + long deadline = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(THREAD_EXIT_MS); + Set alive; + do { + alive = new HashSet<>(Thread.getAllStackTraces().keySet()); + alive.removeAll(before); + alive.removeIf(thread -> !thread.isAlive()); + if (alive.isEmpty()) { + return ""; + } + Thread.sleep(20); + } while (System.nanoTime() < deadline); + return alive.stream().map(Thread::getName).sorted().collect(Collectors.joining(", ")); + } +} diff --git a/src/test/java/org/metricshub/ipmi/core/api/sync/IpmiConnectorTest.java b/src/test/java/org/metricshub/ipmi/core/api/sync/IpmiConnectorTest.java new file mode 100644 index 0000000..e5cd670 --- /dev/null +++ b/src/test/java/org/metricshub/ipmi/core/api/sync/IpmiConnectorTest.java @@ -0,0 +1,59 @@ +package org.metricshub.ipmi.core.api.sync; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import org.junit.jupiter.api.Test; +import org.metricshub.ipmi.core.api.async.ConnectionHandle; +import org.metricshub.ipmi.core.connection.ConnectionException; +import org.metricshub.ipmi.core.transport.FakeBmc; + +class IpmiConnectorTest { + + /** + * RMCP header, then an RMCP+ sessionless, unauthenticated IPMI message that announces an 80-byte payload the + * datagram does not carry: it passes the filters of the cipher-suites step and fails to decode. + */ + private static final byte[] TRUNCATED_REPLY = { + 0x06, + 0x00, + (byte) 0xff, + 0x07, + 0x06, + 0x00, + 0x00, + 0x00, + 0x00, + 0x00, + 0x00, + 0x00, + 0x00, + 0x00, + 0x50, + 0x00 }; + + private static final int TIMEOUT_MS = 500; + + @Test + void aBadReplyFailsTheStepAtOnceAndLeavesItRetriable() throws Exception { + try (FakeBmc bmc = new FakeBmc(request -> TRUNCATED_REPLY)) { + IpmiConnector connector = new IpmiConnector(0); + try { + ConnectionHandle handle = connector.createConnection(bmc.getAddress(), bmc.getPort()); + connector.setTimeout(handle, TIMEOUT_MS); + + Exception first = assertThrows(Exception.class, () -> connector.getAvailableCipherSuites(handle)); + assertFalse(first instanceof ConnectionException, "the decoding failure, not a timeout: " + first); + assertEquals(1, bmc.getRequestCount(), "a reply that is not a timeout must not be sent again"); + + // The state machine was rolled back: the same step can be tried again + Exception second = assertThrows(Exception.class, () -> connector.getAvailableCipherSuites(handle)); + assertFalse(second instanceof ConnectionException, String.valueOf(second)); + assertEquals(2, bmc.getRequestCount()); + } finally { + connector.tearDown(); + } + } + } +} diff --git a/src/test/java/org/metricshub/ipmi/core/transport/FakeBmc.java b/src/test/java/org/metricshub/ipmi/core/transport/FakeBmc.java new file mode 100644 index 0000000..057e717 --- /dev/null +++ b/src/test/java/org/metricshub/ipmi/core/transport/FakeBmc.java @@ -0,0 +1,84 @@ +package org.metricshub.ipmi.core.transport; + +import java.io.IOException; +import java.net.DatagramPacket; +import java.net.DatagramSocket; +import java.net.InetAddress; +import java.net.SocketException; +import java.util.Arrays; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Function; + +/** + * A UDP endpoint on the loopback interface that stands in for a BMC: every datagram it receives is handed to a + * responder whose return value is sent back (or dropped when null). + */ +public class FakeBmc implements AutoCloseable { + + private final DatagramSocket socket; + private final Function responder; + private final AtomicInteger requests = new AtomicInteger(); + + /** + * @param responder computes the reply to a request, or returns null to drop it + * @throws SocketException when no loopback port is free + */ + public FakeBmc(Function responder) throws SocketException { + this.responder = responder; + socket = new DatagramSocket(0, InetAddress.getLoopbackAddress()); + Thread thread = new Thread(this::serve, "fake-bmc"); + thread.setDaemon(true); + thread.start(); + } + + /** + * @return a BMC that never answers + * @throws SocketException when no loopback port is free + */ + public static FakeBmc silent() throws SocketException { + return new FakeBmc(request -> null); + } + + /** + * @return the loopback address the fake BMC listens on + */ + public InetAddress getAddress() { + return socket.getLocalAddress(); + } + + /** + * @return the UDP port the fake BMC listens on + */ + public int getPort() { + return socket.getLocalPort(); + } + + /** + * @return how many datagrams were received so far + */ + public int getRequestCount() { + return requests.get(); + } + + private void serve() { + byte[] buffer = new byte[1024]; + while (!socket.isClosed()) { + DatagramPacket packet = new DatagramPacket(buffer, buffer.length); + try { + socket.receive(packet); + requests.incrementAndGet(); + byte[] reply = responder.apply(Arrays.copyOf(packet.getData(), packet.getLength())); + if (reply != null) { + socket.send(new DatagramPacket(reply, reply.length, packet.getSocketAddress())); + } + } catch (IOException e) { + return; // closed + } + } + } + + @Override + public void close() { + socket.close(); + } +}