diff --git a/hadoop-ozone/tools/pom.xml b/hadoop-ozone/tools/pom.xml index 9e26438e94fa..829a76577b18 100644 --- a/hadoop-ozone/tools/pom.xml +++ b/hadoop-ozone/tools/pom.xml @@ -46,6 +46,10 @@ jakarta.xml.bind jakarta.xml.bind-api + + org.apache.commons + commons-lang3 + org.apache.hadoop hadoop-common diff --git a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/local/LocalOzoneCluster.java b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/local/LocalOzoneCluster.java index 89961e8243ad..b9e93a661712 100644 --- a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/local/LocalOzoneCluster.java +++ b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/local/LocalOzoneCluster.java @@ -92,6 +92,8 @@ import java.util.UUID; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import java.util.function.Supplier; +import java.util.stream.Collectors; import java.util.stream.Stream; import org.apache.hadoop.hdds.client.ReplicationFactor; import org.apache.hadoop.hdds.client.ReplicationType; @@ -153,6 +155,7 @@ public final class LocalOzoneCluster implements LocalOzoneRuntime { private static final int LOCAL_RATIS_RPC_TIMEOUT_SECONDS = 1; private static final long SCM_CLIENT_MAX_RETRY_TIMEOUT_MILLIS = 30_000; private static final long READINESS_POLL_INTERVAL_MILLIS = 500; + private static final long READINESS_LOG_INTERVAL_NANOS = TimeUnit.SECONDS.toNanos(5); private static final int MAX_PORT = 65_535; @@ -186,6 +189,8 @@ public final class LocalOzoneCluster implements LocalOzoneRuntime { // local ports, so an unbounded count would exhaust local ports; cap it. static final int MAX_DATANODES = 20; + private static final String OZONE_DEFAULT_XML = "ozone-default.xml"; + private static final String[] NO_ARGS = new String[0]; private final LocalOzoneClusterConfig config; @@ -196,6 +201,7 @@ public final class LocalOzoneCluster implements LocalOzoneRuntime { private StorageContainerManager scm; private OzoneManager om; private final List datanodes = new ArrayList<>(); + private final List discardedUserConfigKeys = new ArrayList<>(); private boolean previousMetricsMiniClusterMode; private boolean metricsMiniClusterModeEnabled; @@ -239,6 +245,11 @@ PreparedConfiguration prepareConfiguration() throws IOException { return preparedConfiguration; } + // preparedConfiguration is assigned only on success, so a failed attempt can be retried on this + // instance; start from an empty list so a retry does not report an override twice. + discardedUserConfigKeys.clear(); + + requireSupportedDatanodeCount(); prepareStorageLayout(); OzoneConfiguration conf = new OzoneConfiguration(seedConfiguration); @@ -290,6 +301,11 @@ public String getS3Endpoint() { return ""; } + @Override + public List getDiscardedUserConfigKeys() { + return Collections.unmodifiableList(discardedUserConfigKeys); + } + @Override public void close() throws IOException { if (closed) { @@ -314,8 +330,10 @@ public void close() throws IOException { private void stopServices() { try { - // Shutdown is best-effort so one failed service cannot leak the others. - IOUtils.closeQuietly(this::stopDatanodes, this::stopOm, this::stopScm); + // Shutdown is best-effort so one failed service cannot leak the others, + // but the failures are logged: stopServices() also runs as start() + // rollback, where a silent failure hides leaked threads and ports. + IOUtils.close(LOG, this::stopDatanodes, this::stopOm, this::stopScm); } finally { restoreSameJvmMetricsMode(); } @@ -331,7 +349,7 @@ private void stopDatanodes() { }); } datanodes.clear(); - IOUtils.closeQuietly(stoppers); + IOUtils.close(LOG, stoppers); } private void stopOm() { @@ -352,27 +370,25 @@ private void stopScm() { } private void configureLocalDefaults(OzoneConfiguration conf) { - conf.set(OZONE_METADATA_DIRS, metadataDir().toString()); + setLocalOverride(conf, OZONE_METADATA_DIRS, metadataDir().toString()); // SCM and OM share one configuration and JVM, so the Jetty base dir is a // single service-neutral location; Jetty keeps per-context temp dirs. conf.setIfUnset(OZONE_HTTP_BASEDIR, metadataDir() + SERVER_DIR); - conf.set(OZONE_REPLICATION, ReplicationFactor.ONE.name()); - conf.set(OZONE_REPLICATION_TYPE, ReplicationType.STAND_ALONE.name()); - conf.set(OZONE_SERVER_DEFAULT_REPLICATION_KEY, ReplicationFactor.ONE.name()); - conf.set(OZONE_SERVER_DEFAULT_REPLICATION_TYPE_KEY, + setLocalOverride(conf, OZONE_REPLICATION, ReplicationFactor.ONE.name()); + setLocalOverride(conf, OZONE_REPLICATION_TYPE, ReplicationType.STAND_ALONE.name()); + setLocalOverride(conf, OZONE_SERVER_DEFAULT_REPLICATION_KEY, ReplicationFactor.ONE.name()); + setLocalOverride(conf, OZONE_SERVER_DEFAULT_REPLICATION_TYPE_KEY, ReplicationType.STAND_ALONE.name()); - conf.setBoolean(HDDS_CONTAINER_RATIS_ENABLED_KEY, false); + setLocalOverride(conf, HDDS_CONTAINER_RATIS_ENABLED_KEY, "false"); // A single-node local cluster can heartbeat aggressively; this speeds - // datanode registration and safe-mode exit. Use set(), not setIfUnset(): - // ozone-default.xml supplies the 30s default that would otherwise defeat - // the override. - conf.set(HDDS_HEARTBEAT_INTERVAL, "1s"); - conf.setBoolean(HDDS_SCM_SAFEMODE_PIPELINE_CREATION, false); - conf.setInt(HDDS_SCM_SAFEMODE_MIN_DATANODE, - Math.max(1, config.getDatanodes())); - conf.setTimeDuration(OZONE_OM_RATIS_MINIMUM_TIMEOUT_KEY, - LOCAL_RATIS_RPC_TIMEOUT_SECONDS, TimeUnit.SECONDS); - conf.set(HDDS_SCM_WAIT_TIME_AFTER_SAFE_MODE_EXIT, "3s"); + // datanode registration and safe-mode exit. + setLocalOverride(conf, HDDS_HEARTBEAT_INTERVAL, "1s"); + setLocalOverride(conf, HDDS_SCM_SAFEMODE_PIPELINE_CREATION, "false"); + setLocalOverride(conf, HDDS_SCM_SAFEMODE_MIN_DATANODE, + String.valueOf(Math.max(1, config.getDatanodes()))); + setLocalOverride(conf, OZONE_OM_RATIS_MINIMUM_TIMEOUT_KEY, + LOCAL_RATIS_RPC_TIMEOUT_SECONDS + "s"); + setLocalOverride(conf, HDDS_SCM_WAIT_TIME_AFTER_SAFE_MODE_EXIT, "3s"); conf.setIfUnset(OZONE_SCM_HA_RATIS_SERVER_RPC_FIRST_ELECTION_TIMEOUT, "1s"); @@ -381,6 +397,32 @@ private void configureLocalDefaults(OzoneConfiguration conf) { conf.setFromObject(scmClientConfig); } + /** + * Applies a value the local runtime requires, reporting the value it replaces. These keys are + * set rather than {@code setIfUnset} because ozone-default.xml would otherwise win, so a value + * the user put in ozone-site.xml or passed with -D is discarded; reporting happens here, at the + * point of the override, so a new override cannot be added without being reported. + */ + private void setLocalOverride(OzoneConfiguration conf, String key, String value) { + String configured = conf.get(key); + if (!value.equals(configured) && isUserConfigured(conf, key)) { + discardedUserConfigKeys.add(key); + LOG.warn("ozone local requires {}={}; the configured value {} is ignored.", + key, value, configured); + } + conf.set(key, value); + } + + /** + * Whether {@code key} carries a value the user chose. A value whose last source is + * ozone-default.xml is a shipped default, not a user choice. + */ + private static boolean isUserConfigured(OzoneConfiguration conf, String key) { + String[] sources = conf.getPropertySources(key); + return sources != null && sources.length > 0 + && !sources[sources.length - 1].contains(OZONE_DEFAULT_XML); + } + private int configureScm(OzoneConfiguration conf, PersistedPortState persistedPorts, PortAllocator portAllocator) throws IOException { @@ -480,13 +522,6 @@ private void configureOmStorage(OzoneConfiguration conf) throws IOException { private List configureDatanodes(OzoneConfiguration conf, PersistedPortState persistedPorts, PortAllocator portAllocator) throws IOException { - int datanodeCount = config.getDatanodes(); - if (datanodeCount > MAX_DATANODES) { - throw new IOException("Datanode count " + datanodeCount - + " exceeds the local maximum of " + MAX_DATANODES - + "; each datanode reserves " + DATANODE_PORT_KEY_SUFFIXES.length - + " local ports."); - } List datanodeConfigurations = new ArrayList<>(config.getDatanodes()); for (int index = 0; index < config.getDatanodes(); index++) { @@ -643,30 +678,66 @@ private void startDatanodes(List datanodeConfigurations) { } private void waitForClusterReadiness(Duration timeout) throws Exception { + waitForReadiness(this::clusterReadinessBlocker, "Ozone cluster", timeout); + } + + /** + * Polls {@code blocker} until it reports ready. {@code blocker} returns why {@code subject} is + * not ready yet, so the wait can name the unmet condition instead of reporting a bare timeout + * that cannot distinguish a slow start from a stuck one. + */ + static void waitForReadiness(Supplier blocker, String subject, Duration timeout) + throws InterruptedException, TimeoutException { long deadlineNanos = System.nanoTime() + timeout.toNanos(); + long nextLogNanos = System.nanoTime() + READINESS_LOG_INTERVAL_NANOS; while (true) { - if (isClusterReady()) { + String reason = blocker.get(); + if (reason == null) { return; } if (System.nanoTime() >= deadlineNanos) { - throw new TimeoutException("Timed out waiting " + timeout - + " for the local Ozone cluster to become ready."); + throw new TimeoutException("Timed out waiting " + timeout + " for the local " + subject + + " to become ready: " + reason + "."); + } + if (System.nanoTime() >= nextLogNanos) { + LOG.info("Waiting for the local {} to become ready: {}.", subject, reason); + nextLogNanos = System.nanoTime() + READINESS_LOG_INTERVAL_NANOS; } Thread.sleep(READINESS_POLL_INTERVAL_MILLIS); } } - private boolean isClusterReady() { - if (!scm.checkLeader() || !om.isLeaderReady()) { - return false; + /** + * Returns why the cluster is not usable yet, or null once it is ready. The cluster is usable + * once SCM and OM are leader-ready, every datanode has registered with SCM, and SCM has left + * safe mode. + */ + private String clusterReadinessBlocker() { + if (!scm.checkLeader()) { + return "SCM has no Ratis leader yet"; } - if (config.getDatanodes() == 0) { - return true; + if (!om.isLeaderReady()) { + return "OM is not leader-ready yet"; } - // The cluster is usable once every datanode has registered with SCM and - // SCM has left safe mode. - return scm.getScmNodeManager().getAllNodes().size() >= config.getDatanodes() - && !scm.isInSafeMode(); + int registered = scm.getScmNodeManager().getAllNodes().size(); + if (registered < config.getDatanodes()) { + return "only " + registered + " of " + config.getDatanodes() + + " datanodes have registered with SCM"; + } + // Safe mode is checked even for a zero-datanode cluster: configureLocalDefaults() still + // requires one datanode there, so skipping the check would report a cluster that can never + // serve requests as ready. + if (scm.isInSafeMode()) { + return "SCM is still in safe mode (" + unmetSafeModeRules() + ")"; + } + return null; + } + + private String unmetSafeModeRules() { + return scm.getScmSafeModeManager().getRuleStatus().entrySet().stream() + .filter(rule -> !rule.getValue().getLeft()) + .map(rule -> rule.getKey() + ": " + rule.getValue().getRight()) + .collect(Collectors.joining("; ")); } private void enableSameJvmMetricsMode() { @@ -684,6 +755,21 @@ private void restoreSameJvmMetricsMode() { } } + /** + * Rejects a datanode count this host cannot serve. Called before {@link #prepareStorageLayout()} + * because format mode ALWAYS deletes the data dir: validating afterwards would destroy local + * state for a run that cannot start anyway. + */ + private void requireSupportedDatanodeCount() throws IOException { + int datanodeCount = config.getDatanodes(); + if (datanodeCount > MAX_DATANODES) { + throw new IOException("Datanode count " + datanodeCount + + " exceeds the local maximum of " + MAX_DATANODES + + "; each datanode reserves " + DATANODE_PORT_KEY_SUFFIXES.length + + " local ports."); + } + } + private void prepareStorageLayout() throws IOException { Path dataDir = config.getDataDir(); if (Files.exists(dataDir) && !Files.isDirectory(dataDir)) { @@ -693,6 +779,7 @@ private void prepareStorageLayout() throws IOException { switch (config.getFormatMode()) { case ALWAYS: + LOG.info("Removing local Ozone data dir {} (format mode ALWAYS).", dataDir); deleteDirectory(dataDir); createBaseLayout(); break; diff --git a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/local/LocalOzoneRuntime.java b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/local/LocalOzoneRuntime.java index 3ed939044176..2c4c0edc9eb7 100644 --- a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/local/LocalOzoneRuntime.java +++ b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/local/LocalOzoneRuntime.java @@ -17,6 +17,8 @@ package org.apache.hadoop.ozone.local; +import java.util.List; + /** * Runtime contract for local Ozone cluster commands. */ @@ -71,6 +73,16 @@ public interface LocalOzoneRuntime extends AutoCloseable { */ String getS3Endpoint(); + /** + * Returns the configuration keys whose user-supplied value the local runtime had to replace. + * + *

Callers report these: {@code ozone local} runs with service logging off, so a warning that + * only reaches the log is invisible by default.

+ * + * @return overridden keys, empty if the user configured none of them + */ + List getDiscardedUserConfigKeys(); + /** * Stops the local runtime and releases resources created during startup. * diff --git a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/local/OzoneLocal.java b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/local/OzoneLocal.java index 0d5e6ec17345..231b285d5a08 100644 --- a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/local/OzoneLocal.java +++ b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/local/OzoneLocal.java @@ -17,10 +17,12 @@ package org.apache.hadoop.ozone.local; +import java.io.IOException; import java.io.PrintWriter; import java.nio.file.Path; import java.time.Duration; import java.time.format.DateTimeParseException; +import java.util.List; import java.util.concurrent.Callable; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -197,13 +199,58 @@ static class RunCommand extends AbstractSubcommand implements Callable { public Void call() throws Exception { LocalOzoneClusterConfig config = resolveConfig(); try (LocalOzoneRuntime runtime = createRuntime(config, getOzoneConf())) { - runtime.start(); + start(runtime); printSummary(runtime, config); awaitShutdown(runtime); } return null; } + /** + * Starts {@code runtime}, restating a failure in a form the user can act on. Service logs are + * off by default for this command, so the detail goes to the log for {@code --loglevel INFO} + * while the message keeps {@link GenericCli}'s single-line path: an exception with no message + * would otherwise print a raw stack trace. + */ + private void start(LocalOzoneRuntime runtime) throws Exception { + try { + runtime.start(); + } catch (Exception ex) { + LOG.error("Local Ozone cluster failed to start.", ex); + throw new IOException("Local Ozone failed to start: " + failureMessage(ex) + + " Re-run with `ozone --loglevel INFO local run` for service logs," + + " or add --verbose for the full stack trace.", ex); + } finally { + reportDiscardedConfig(runtime); + } + } + + /** + * Reports configuration the local runtime had to replace. The runtime also logs each + * replacement, but service logging is off by default for this command, so the keys are + * repeated here where the user will actually see them. Reported even when startup fails: + * a discarded override can be the very setting the user is debugging. + */ + private void reportDiscardedConfig(LocalOzoneRuntime runtime) { + List discarded = runtime.getDiscardedUserConfigKeys(); + if (!discarded.isEmpty()) { + err().println("Ignoring configured " + String.join(", ", discarded) + + ": ozone local requires its own values for these." + + " Re-run with `ozone --loglevel INFO local run` to see them."); + } + } + + /** Returns the nearest message in {@code error}'s cause chain, since the outermost may be null. */ + private static String failureMessage(Throwable error) { + for (Throwable cause = error; cause != null; cause = cause.getCause()) { + String message = cause.getMessage(); + if (message != null && !message.isEmpty()) { + return message.endsWith(".") ? message : message + "."; + } + } + return error.getClass().getSimpleName() + "."; + } + LocalOzoneRuntime createRuntime(LocalOzoneClusterConfig config, OzoneConfiguration seedConfiguration) { return new LocalOzoneCluster(config, seedConfiguration); } @@ -213,6 +260,7 @@ private void printSummary(LocalOzoneRuntime runtime, LocalOzoneClusterConfig con writer.println("Local Ozone is running from " + config.getDataDir()); writer.println("SCM RPC: " + runtime.getDisplayHost() + ":" + runtime.getScmPort()); writer.println("OM RPC: " + runtime.getDisplayHost() + ":" + runtime.getOmPort()); + writer.println("Datanodes: " + config.getDatanodes()); writer.println("Press Ctrl+C to stop."); writer.flush(); } @@ -293,8 +341,11 @@ public LocalOzoneClusterConfig.FormatMode convert(String value) { try { return LocalOzoneClusterConfig.FormatMode.fromString(value); } catch (IllegalArgumentException ex) { - throw new CommandLine.TypeConversionException( - "Expected one of: if-needed, always, never."); + // The value can come from OZONE_LOCAL_FORMAT, so name it: the user may not realize the + // environment supplied it. picocli's conversion-error line names the option but not the + // value, so the message has to carry it. + throw new CommandLine.TypeConversionException("Invalid format mode '" + value + + "'. Expected one of: if-needed, always, never."); } } } @@ -304,19 +355,29 @@ private static final class DurationConverter @Override public Duration convert(String value) { + String trimmed = value.trim(); try { - return Duration.parse(value.trim()); + return Duration.parse(trimmed); } catch (DateTimeParseException ignored) { - return parseHadoopStyleDuration(value); + return parseHadoopStyleDuration(trimmed); } } private static Duration parseHadoopStyleDuration(String value) { + // TimeDurationUtil.getDuration() only warns about a missing unit and then assumes the one + // it is given, which would read "120" as 120 milliseconds. Reject it instead of silently + // interpreting the value a thousand times smaller than the user meant. Every unit suffix + // TimeDurationUtil accepts ends in a letter, so a trailing digit means the unit is missing. + if (!value.isEmpty() && Character.isDigit(value.charAt(value.length() - 1))) { + throw new CommandLine.TypeConversionException("Missing time unit in '" + value + + "'. " + durationMessage()); + } try { return TimeDurationUtil.getDuration("--startup-timeout", value, TimeUnit.MILLISECONDS); } catch (RuntimeException ex) { - throw new CommandLine.TypeConversionException(durationMessage()); + throw new CommandLine.TypeConversionException("Invalid duration '" + value + + "'. " + durationMessage()); } } diff --git a/hadoop-ozone/tools/src/test/java/org/apache/hadoop/ozone/local/TestLocalOzoneCluster.java b/hadoop-ozone/tools/src/test/java/org/apache/hadoop/ozone/local/TestLocalOzoneCluster.java index 1f109c6de420..eb9de7edfce3 100644 --- a/hadoop-ozone/tools/src/test/java/org/apache/hadoop/ozone/local/TestLocalOzoneCluster.java +++ b/hadoop-ozone/tools/src/test/java/org/apache/hadoop/ozone/local/TestLocalOzoneCluster.java @@ -18,6 +18,7 @@ package org.apache.hadoop.ozone.local; import static java.nio.charset.StandardCharsets.UTF_8; +import static java.util.Collections.singletonList; import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_SCM_SAFEMODE_MIN_DATANODE; import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_SCM_SAFEMODE_PIPELINE_CREATION; import static org.apache.hadoop.hdds.scm.ScmConfigKeys.HDDS_CONTAINER_RATIS_ENABLED_KEY; @@ -42,10 +43,14 @@ import java.io.InputStream; import java.nio.file.Files; import java.nio.file.Path; +import java.time.Duration; import java.util.Properties; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicInteger; import org.apache.hadoop.hdds.client.ReplicationFactor; import org.apache.hadoop.hdds.client.ReplicationType; import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.ozone.test.GenericTestUtils.LogCapturer; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -146,6 +151,27 @@ void prepareConfigurationIsIdempotent() throws Exception { } } + @Test + void retryAfterFailedPrepareDoesNotRepeatDiscardedKeys() throws Exception { + OzoneConfiguration seed = new OzoneConfiguration(); + seed.set(OZONE_REPLICATION, ReplicationFactor.THREE.name()); + // Duplicate ports fail in configureOm(), after configureLocalDefaults() has recorded the + // override, so the second attempt re-runs the recording. + LocalOzoneClusterConfig config = LocalOzoneClusterConfig.builder( + tempDir.resolve("local-ozone")) + .setScmPort(9860) + .setOmPort(9860) + .build(); + + try (LocalOzoneCluster cluster = new LocalOzoneCluster(config, seed)) { + assertThrows(IOException.class, cluster::prepareConfiguration); + assertThrows(IOException.class, cluster::prepareConfiguration); + + assertEquals(singletonList(OZONE_REPLICATION), + cluster.getDiscardedUserConfigKeys()); + } + } + @Test void prepareConfigurationRejectsDuplicateConfiguredPorts() { LocalOzoneClusterConfig config = LocalOzoneClusterConfig.builder( @@ -349,6 +375,79 @@ void prepareConfigurationRejectsTooManyDatanodes() throws Exception { + "; each datanode reserves 8 local ports.", error.getMessage()); } + @Test + void tooManyDatanodesIsRejectedBeforeFormatDeletesDataDir() throws Exception { + Path dataDir = tempDir.resolve("local-ozone"); + Path marker = writeMarker(dataDir, "keep me"); + LocalOzoneClusterConfig config = LocalOzoneClusterConfig.builder(dataDir) + .setFormatMode(LocalOzoneClusterConfig.FormatMode.ALWAYS) + .setDatanodes(LocalOzoneCluster.MAX_DATANODES + 1) + .build(); + + assertPrepareFails(config); + + assertTrue(Files.exists(marker), + "format ALWAYS must not delete the data dir for a run that cannot start"); + } + + @Test + void forcedLocalDefaultWarnsAboutDiscardedUserValue() throws Exception { + OzoneConfiguration seed = new OzoneConfiguration(); + seed.set(OZONE_REPLICATION, ReplicationFactor.THREE.name()); + LocalOzoneClusterConfig config = LocalOzoneClusterConfig.builder( + tempDir.resolve("local-ozone")).build(); + + LogCapturer logs = LogCapturer.captureLogs(LocalOzoneCluster.class); + try (LocalOzoneCluster cluster = new LocalOzoneCluster(config, seed)) { + cluster.prepareConfiguration(); + + assertTrue(logs.getOutput().contains(OZONE_REPLICATION), logs.getOutput()); + assertTrue(logs.getOutput().contains(ReplicationFactor.THREE.name()), + logs.getOutput()); + // The CLI repeats these, because the log above is off by default for ozone local. + assertEquals(singletonList(OZONE_REPLICATION), + cluster.getDiscardedUserConfigKeys()); + } finally { + logs.stopCapturing(); + } + } + + @Test + void forcedLocalDefaultIsQuietWhenUserConfiguredNothing() throws Exception { + LocalOzoneClusterConfig config = LocalOzoneClusterConfig.builder( + tempDir.resolve("local-ozone")).build(); + + try (LocalOzoneCluster cluster = new LocalOzoneCluster(config, + new OzoneConfiguration())) { + cluster.prepareConfiguration(); + + assertTrue(cluster.getDiscardedUserConfigKeys().isEmpty(), + cluster.getDiscardedUserConfigKeys().toString()); + } + } + + @Test + void readinessTimeoutNamesTheUnmetCondition() { + TimeoutException error = assertThrows(TimeoutException.class, + () -> LocalOzoneCluster.waitForReadiness(() -> "only 1 of 3 datanodes have registered", + "Ozone cluster", Duration.ofMillis(1))); + + assertTrue(error.getMessage().contains("only 1 of 3 datanodes have registered"), + error.getMessage()); + assertTrue(error.getMessage().contains("Ozone cluster"), error.getMessage()); + } + + @Test + void readinessReturnsOnceBlockerReportsReady() throws Exception { + AtomicInteger attempts = new AtomicInteger(); + + LocalOzoneCluster.waitForReadiness( + () -> attempts.incrementAndGet() < 2 ? "not yet" : null, "Ozone cluster", + Duration.ofSeconds(30)); + + assertEquals(2, attempts.get()); + } + @Test void persistedPortFileContainsDatanodePorts() throws Exception { Path dataDir = tempDir.resolve("local-ozone"); diff --git a/hadoop-ozone/tools/src/test/java/org/apache/hadoop/ozone/local/TestOzoneLocal.java b/hadoop-ozone/tools/src/test/java/org/apache/hadoop/ozone/local/TestOzoneLocal.java index 0f8686e91a2b..306b2638acc0 100644 --- a/hadoop-ozone/tools/src/test/java/org/apache/hadoop/ozone/local/TestOzoneLocal.java +++ b/hadoop-ozone/tools/src/test/java/org/apache/hadoop/ozone/local/TestOzoneLocal.java @@ -18,18 +18,23 @@ package org.apache.hadoop.ozone.local; import static java.nio.charset.StandardCharsets.UTF_8; +import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.ByteArrayOutputStream; +import java.io.IOException; import java.io.OutputStreamWriter; import java.io.PrintWriter; import java.lang.reflect.Field; import java.nio.file.Paths; import java.time.Duration; +import java.util.List; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.junit.jupiter.api.Test; import picocli.CommandLine; @@ -123,6 +128,60 @@ void runCommandClosesRuntimeWhenStartupFails() { assertTrue(runtime.closed); } + @Test + void runCommandReportsDiscardedConfigToStdErr() throws Exception { + ByteArrayOutputStream err = new ByteArrayOutputStream(); + StubRuntime runtime = new StubRuntime("localhost", 9860, 9862); + runtime.discardedUserConfigKeys = singletonList("ozone.replication"); + TestableRunCommand command = new TestableRunCommand(runtime); + CommandLine commandLine = new CommandLine(command); + commandLine.setErr(new PrintWriter(new OutputStreamWriter(err, UTF_8), true)); + + int exitCode = commandLine.execute(); + + assertEquals(0, exitCode); + // Service logging is off by default for this command, so the CLI has to say it itself. + assertTrue(err.toString(UTF_8.name()).contains("ozone.replication"), + err.toString(UTF_8.name())); + } + + @Test + void runCommandReportsDiscardedConfigWhenStartupFails() throws Exception { + ByteArrayOutputStream err = new ByteArrayOutputStream(); + StubRuntime runtime = new StubRuntime("localhost", 9860, 9862); + runtime.failStart = true; + runtime.discardedUserConfigKeys = singletonList("ozone.replication"); + TestableRunCommand command = new TestableRunCommand(runtime); + CommandLine commandLine = new CommandLine(command); + commandLine.setErr(new PrintWriter(new OutputStreamWriter(err, UTF_8), true)); + + int exitCode = commandLine.execute(); + + assertEquals(1, exitCode); + // A discarded override can be the very setting that made startup fail, so it is reported + // even on the failure path. + assertTrue(err.toString(UTF_8.name()).contains("ozone.replication"), + err.toString(UTF_8.name())); + } + + @Test + void runCommandStartupFailureReportsCauseAndHowToGetDetail() { + StubRuntime runtime = new StubRuntime("localhost", 9860, 9862); + runtime.failStart = true; + TestableRunCommand command = new TestableRunCommand(runtime); + new CommandLine(command).parseArgs(); + + IOException error = assertThrows(IOException.class, command::call); + + // GenericCli prints only the first line of the message, so it has to carry the cause itself. + String message = error.getMessage(); + assertTrue(message.contains("startup failed"), message); + assertTrue(message.contains("--loglevel INFO"), message); + assertTrue(message.contains("--verbose"), message); + assertInstanceOf(IllegalStateException.class, error.getCause()); + assertTrue(runtime.closed); + } + @Test void runCommandOptionsUseEnvironmentDefaults() throws Exception { assertEnvDefault("dataDir", OzoneLocal.ENV_DATA_DIR, @@ -250,7 +309,9 @@ void resolveConfigRejectsDatanodeCountBelowOne() { @Test void resolveConfigRejectsInvalidDuration() { - assertParseError("--startup-timeout", "forever", "--startup-timeout"); + // Pins the value echo: picocli's wrapper already names the option, so asserting on the + // option alone would pass even if the converter dropped the value from its message. + assertParseError("--startup-timeout", "forever", "Invalid duration 'forever'"); } @Test @@ -258,6 +319,25 @@ void resolveConfigRejectsNonPositiveDuration() { assertConfigError("--startup-timeout", "0s", "--startup-timeout"); } + @Test + void resolveConfigRejectsDurationWithoutTimeUnit() { + // Without the unit check this parses as 120 milliseconds, so the run dies with an unrelated + // timeout instead of telling the user the value was misread. Asserts the quoted value: the + // bare digits also occur in the static "like 120s" hint, which would mask a dropped echo. + assertParseError("--startup-timeout", "120", "Missing time unit in '120'"); + } + + @Test + void resolveConfigAcceptsHadoopStyleMinutes() { + assertEquals(Duration.ofMinutes(2), resolve("--startup-timeout", "2m") + .getStartupTimeout()); + } + + @Test + void invalidFormatModeMessageNamesOffendingValue() { + assertParseError("--format", "sometimes", "sometimes"); + } + @Test void resolveConfigRejectsInvalidPath() { assertParseError("--data-dir", "\0", "--data-dir"); @@ -364,6 +444,7 @@ private static final class StubRuntime implements LocalOzoneRuntime { private final int omPort; private boolean failStart; private boolean started; + private List discardedUserConfigKeys = emptyList(); private boolean closed; private StubRuntime(String displayHost, int scmPort, int omPort) { @@ -405,6 +486,11 @@ public String getS3Endpoint() { return ""; } + @Override + public List getDiscardedUserConfigKeys() { + return discardedUserConfigKeys; + } + @Override public void close() { closed = true;