From 8e178c4f8e0321929b4147c1c6e1d22e488bea5d Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Mon, 2 May 2016 10:59:50 -0400 Subject: [PATCH] Add system bootstrap checks escape hatch Today when running in production mode the bootstrap checks are completely unforgiving. But there are cases where an end-user might not have the ability to modify some of the system-level settings that cause the bootstrap checks to trip (e.g., guest settings that are inherited from a host and can not be modified). This commit adds a setting that allows system-level bootstrap checks to be ignored for these end-users. We classify certain bootstrap checks into system-level checks and only those bootstrap checks will be ignored if this flag is enabled. All other bootstrap checks are still subject to being enforced if the user is in production mode. We will still log warnings for these bootstrap checks because the end-user does still need to be made aware that they are running in a configuration that is less-than-ideal from a resiliency perspective. Relates #18088 --- .../bootstrap/BootstrapCheck.java | 121 +++++++++++++++--- .../bootstrap/BootstrapSettings.java | 2 + .../common/settings/ClusterSettings.java | 1 + .../bootstrap/BootstrapCheckTests.java | 90 ++++++++++--- 4 files changed, 175 insertions(+), 39 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/bootstrap/BootstrapCheck.java b/core/src/main/java/org/elasticsearch/bootstrap/BootstrapCheck.java index 2f87086ede4..25bdbe3fa8a 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/BootstrapCheck.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/BootstrapCheck.java @@ -41,7 +41,6 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Locale; -import java.util.stream.Collectors; /** * We enforce limits once any network host is configured. In this case we assume the node is running in production @@ -63,42 +62,80 @@ final class BootstrapCheck { * @param boundTransportAddress the node network bindings */ static void check(final Settings settings, final BoundTransportAddress boundTransportAddress) { - check(enforceLimits(boundTransportAddress), checks(settings), Node.NODE_NAME_SETTING.get(settings)); + check( + enforceLimits(boundTransportAddress), + BootstrapSettings.IGNORE_SYSTEM_BOOTSTRAP_CHECKS.get(settings), + checks(settings), + Node.NODE_NAME_SETTING.get(settings)); } /** * executes the provided checks and fails the node if * enforceLimits is true, otherwise logs warnings * - * @param enforceLimits true if the checks should be enforced or - * warned - * @param checks the checks to execute - * @param nodeName the node name to be used as a logging prefix + * @param enforceLimits true if the checks should be enforced or + * otherwise warned + * @param ignoreSystemChecks true if system checks should be enforced + * or otherwise warned + * @param checks the checks to execute + * @param nodeName the node name to be used as a logging prefix */ // visible for testing - static void check(final boolean enforceLimits, final List checks, final String nodeName) { - final ESLogger logger = Loggers.getLogger(BootstrapCheck.class, nodeName); + static void check(final boolean enforceLimits, final boolean ignoreSystemChecks, final List checks, final String nodeName) { + check(enforceLimits, ignoreSystemChecks, checks, Loggers.getLogger(BootstrapCheck.class, nodeName)); + } - final List errors = - checks.stream() - .filter(BootstrapCheck.Check::check) - .map(BootstrapCheck.Check::errorMessage) - .collect(Collectors.toList()); + /** + * executes the provided checks and fails the node if + * enforceLimits is true, otherwise logs warnings + * + * @param enforceLimits true if the checks should be enforced or + * otherwise warned + * @param ignoreSystemChecks true if system checks should be enforced + * or otherwise warned + * @param checks the checks to execute + * @param logger the logger to + */ + static void check( + final boolean enforceLimits, + final boolean ignoreSystemChecks, + final List checks, + final ESLogger logger) { + final List errors = new ArrayList<>(); + final List ignoredErrors = new ArrayList<>(); - if (!errors.isEmpty()) { - final List messages = new ArrayList<>(1 + errors.size()); - messages.add("bootstrap checks failed"); - messages.addAll(errors); - if (enforceLimits) { + for (final Check check : checks) { + if (check.check()) { + if (!enforceLimits || (check.isSystemCheck() && ignoreSystemChecks)) { + ignoredErrors.add(check.errorMessage()); + } else { + errors.add(check.errorMessage()); + } + } + } + + if (!errors.isEmpty() || !ignoredErrors.isEmpty()) { + + if (!ignoredErrors.isEmpty()) { + ignoredErrors.forEach(error -> log(logger, error)); + } + + if (!errors.isEmpty()) { + final List messages = new ArrayList<>(1 + errors.size()); + messages.add("bootstrap checks failed"); + messages.addAll(errors); final RuntimeException re = new RuntimeException(String.join("\n", messages)); errors.stream().map(IllegalStateException::new).forEach(re::addSuppressed); throw re; - } else { - messages.forEach(message -> logger.warn(message)); } + } } + static void log(final ESLogger logger, final String error) { + logger.warn(error); + } + /** * Tests if the checks should be enforced * @@ -151,6 +188,14 @@ final class BootstrapCheck { */ String errorMessage(); + /** + * test if the check is a system-level check + * + * @return true if the check is a system-level check as opposed + * to an Elasticsearch-level check + */ + boolean isSystemCheck(); + } static class HeapSizeCheck implements BootstrapCheck.Check { @@ -183,6 +228,11 @@ final class BootstrapCheck { return JvmInfo.jvmInfo().getConfiguredMaxHeapSize(); } + @Override + public final boolean isSystemCheck() { + return false; + } + } static class OsXFileDescriptorCheck extends FileDescriptorCheck { @@ -233,6 +283,11 @@ final class BootstrapCheck { return ProcessProbe.getInstance().getMaxFileDescriptorCount(); } + @Override + public final boolean isSystemCheck() { + return true; + } + } // visible for testing @@ -259,6 +314,11 @@ final class BootstrapCheck { return Natives.isMemoryLocked(); } + @Override + public final boolean isSystemCheck() { + return true; + } + } static class MinMasterNodesCheck implements Check { @@ -279,6 +339,12 @@ final class BootstrapCheck { return "please set [" + ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING.getKey() + "] to a majority of the number of master eligible nodes in your cluster."; } + + @Override + public final boolean isSystemCheck() { + return false; + } + } static class MaxNumberOfThreadsCheck implements Check { @@ -305,6 +371,11 @@ final class BootstrapCheck { return JNANatives.MAX_NUMBER_OF_THREADS; } + @Override + public final boolean isSystemCheck() { + return true; + } + } static class MaxSizeVirtualMemoryCheck implements Check { @@ -333,6 +404,11 @@ final class BootstrapCheck { return JNANatives.MAX_SIZE_VIRTUAL_MEMORY; } + @Override + public final boolean isSystemCheck() { + return true; + } + } static class MaxMapCountCheck implements Check { @@ -396,6 +472,11 @@ final class BootstrapCheck { return Long.parseLong(procSysVmMaxMapCount); } + @Override + public boolean isSystemCheck() { + return true; + } + } } diff --git a/core/src/main/java/org/elasticsearch/bootstrap/BootstrapSettings.java b/core/src/main/java/org/elasticsearch/bootstrap/BootstrapSettings.java index 4e9dffc995b..b2059380181 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/BootstrapSettings.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/BootstrapSettings.java @@ -37,5 +37,7 @@ public final class BootstrapSettings { Setting.boolSetting("bootstrap.seccomp", true, Property.NodeScope); public static final Setting CTRLHANDLER_SETTING = Setting.boolSetting("bootstrap.ctrlhandler", true, Property.NodeScope); + public static final Setting IGNORE_SYSTEM_BOOTSTRAP_CHECKS = + Setting.boolSetting("bootstrap.ignore_system_bootstrap_checks", false, Property.NodeScope); } diff --git a/core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java b/core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java index 55aa58ca588..dad5f48ce27 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java +++ b/core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java @@ -408,6 +408,7 @@ public final class ClusterSettings extends AbstractScopedSettings { BootstrapSettings.MLOCKALL_SETTING, BootstrapSettings.SECCOMP_SETTING, BootstrapSettings.CTRLHANDLER_SETTING, + BootstrapSettings.IGNORE_SYSTEM_BOOTSTRAP_CHECKS, IndexingMemoryController.INDEX_BUFFER_SIZE_SETTING, IndexingMemoryController.MIN_INDEX_BUFFER_SIZE_SETTING, IndexingMemoryController.MAX_INDEX_BUFFER_SIZE_SETTING, diff --git a/core/src/test/java/org/elasticsearch/bootstrap/BootstrapCheckTests.java b/core/src/test/java/org/elasticsearch/bootstrap/BootstrapCheckTests.java index 9f6a4e25eb5..235957ac18b 100644 --- a/core/src/test/java/org/elasticsearch/bootstrap/BootstrapCheckTests.java +++ b/core/src/test/java/org/elasticsearch/bootstrap/BootstrapCheckTests.java @@ -20,6 +20,7 @@ package org.elasticsearch.bootstrap; import org.apache.lucene.util.Constants; +import org.elasticsearch.common.logging.ESLogger; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.transport.BoundTransportAddress; import org.elasticsearch.common.transport.TransportAddress; @@ -38,6 +39,8 @@ import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.Matchers.hasToString; import static org.hamcrest.Matchers.not; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; public class BootstrapCheckTests extends ESTestCase { @@ -113,6 +116,11 @@ public class BootstrapCheckTests extends ESTestCase { public String errorMessage() { return "first"; } + + @Override + public boolean isSystemCheck() { + return false; + } }, new BootstrapCheck.Check() { @Override @@ -124,11 +132,16 @@ public class BootstrapCheckTests extends ESTestCase { public String errorMessage() { return "second"; } + + @Override + public boolean isSystemCheck() { + return false; + } } ); final RuntimeException e = - expectThrows(RuntimeException.class, () -> BootstrapCheck.check(true, checks, "testExceptionAggregation")); + expectThrows(RuntimeException.class, () -> BootstrapCheck.check(true, false, checks, "testExceptionAggregation")); assertThat(e, hasToString(allOf(containsString("bootstrap checks failed"), containsString("first"), containsString("second")))); final Throwable[] suppressed = e.getSuppressed(); assertThat(suppressed.length, equalTo(2)); @@ -159,7 +172,7 @@ public class BootstrapCheckTests extends ESTestCase { final RuntimeException e = expectThrows( RuntimeException.class, - () -> BootstrapCheck.check(true, Collections.singletonList(check), "testHeapSizeCheck")); + () -> BootstrapCheck.check(true, false, Collections.singletonList(check), "testHeapSizeCheck")); assertThat( e.getMessage(), containsString("initial heap size [" + initialHeapSize.get() + "] " + @@ -167,7 +180,7 @@ public class BootstrapCheckTests extends ESTestCase { initialHeapSize.set(maxHeapSize.get()); - BootstrapCheck.check(true, Collections.singletonList(check), "testHeapSizeCheck"); + BootstrapCheck.check(true, false, Collections.singletonList(check), "testHeapSizeCheck"); // nothing should happen if the initial heap size or the max // heap size is not available @@ -176,7 +189,7 @@ public class BootstrapCheckTests extends ESTestCase { } else { maxHeapSize.set(0); } - BootstrapCheck.check(true, Collections.singletonList(check), "testHeapSizeCheck"); + BootstrapCheck.check(true, false, Collections.singletonList(check), "testHeapSizeCheck"); } public void testFileDescriptorLimits() { @@ -202,17 +215,17 @@ public class BootstrapCheckTests extends ESTestCase { final RuntimeException e = expectThrows(RuntimeException.class, - () -> BootstrapCheck.check(true, Collections.singletonList(check), "testFileDescriptorLimits")); + () -> BootstrapCheck.check(true, false, Collections.singletonList(check), "testFileDescriptorLimits")); assertThat(e.getMessage(), containsString("max file descriptors")); maxFileDescriptorCount.set(randomIntBetween(limit + 1, Integer.MAX_VALUE)); - BootstrapCheck.check(true, Collections.singletonList(check), "testFileDescriptorLimits"); + BootstrapCheck.check(true, false, Collections.singletonList(check), "testFileDescriptorLimits"); // nothing should happen if current file descriptor count is // not available maxFileDescriptorCount.set(-1); - BootstrapCheck.check(true, Collections.singletonList(check), "testFileDescriptorLimits"); + BootstrapCheck.check(true, false, Collections.singletonList(check), "testFileDescriptorLimits"); } public void testFileDescriptorLimitsThrowsOnInvalidLimit() { @@ -255,13 +268,17 @@ public class BootstrapCheckTests extends ESTestCase { if (testCase.shouldFail) { final RuntimeException e = expectThrows( RuntimeException.class, - () -> BootstrapCheck.check(true, Collections.singletonList(check), "testFileDescriptorLimitsThrowsOnInvalidLimit")); + () -> BootstrapCheck.check( + true, + false, + Collections.singletonList(check), + "testFileDescriptorLimitsThrowsOnInvalidLimit")); assertThat( e.getMessage(), containsString("memory locking requested for elasticsearch process but memory is not locked")); } else { // nothing should happen - BootstrapCheck.check(true, Collections.singletonList(check), "testFileDescriptorLimitsThrowsOnInvalidLimit"); + BootstrapCheck.check(true, false, Collections.singletonList(check), "testFileDescriptorLimitsThrowsOnInvalidLimit"); } } } @@ -278,17 +295,17 @@ public class BootstrapCheckTests extends ESTestCase { final RuntimeException e = expectThrows( RuntimeException.class, - () -> BootstrapCheck.check(true, Collections.singletonList(check), "testMaxNumberOfThreadsCheck")); + () -> BootstrapCheck.check(true, false, Collections.singletonList(check), "testMaxNumberOfThreadsCheck")); assertThat(e.getMessage(), containsString("max number of threads")); maxNumberOfThreads.set(randomIntBetween(limit + 1, Integer.MAX_VALUE)); - BootstrapCheck.check(true, Collections.singletonList(check), "testMaxNumberOfThreadsCheck"); + BootstrapCheck.check(true, false, Collections.singletonList(check), "testMaxNumberOfThreadsCheck"); // nothing should happen if current max number of threads is // not available maxNumberOfThreads.set(-1); - BootstrapCheck.check(true, Collections.singletonList(check), "testMaxNumberOfThreadsCheck"); + BootstrapCheck.check(true, false, Collections.singletonList(check), "testMaxNumberOfThreadsCheck"); } public void testMaxSizeVirtualMemory() { @@ -309,17 +326,17 @@ public class BootstrapCheckTests extends ESTestCase { final RuntimeException e = expectThrows( RuntimeException.class, - () -> BootstrapCheck.check(true, Collections.singletonList(check), "testMaxSizeVirtualMemory")); + () -> BootstrapCheck.check(true, false, Collections.singletonList(check), "testMaxSizeVirtualMemory")); assertThat(e.getMessage(), containsString("max size virtual memory")); maxSizeVirtualMemory.set(rlimInfinity); - BootstrapCheck.check(true, Collections.singletonList(check), "testMaxSizeVirtualMemory"); + BootstrapCheck.check(true, false, Collections.singletonList(check), "testMaxSizeVirtualMemory"); // nothing should happen if max size virtual memory is not // available maxSizeVirtualMemory.set(Long.MIN_VALUE); - BootstrapCheck.check(true, Collections.singletonList(check), "testMaxSizeVirtualMemory"); + BootstrapCheck.check(true, false, Collections.singletonList(check), "testMaxSizeVirtualMemory"); } public void testMaxMapCountCheck() { @@ -334,17 +351,17 @@ public class BootstrapCheckTests extends ESTestCase { RuntimeException e = expectThrows( RuntimeException.class, - () -> BootstrapCheck.check(true, Collections.singletonList(check), "testMaxMapCountCheck")); + () -> BootstrapCheck.check(true, false, Collections.singletonList(check), "testMaxMapCountCheck")); assertThat(e.getMessage(), containsString("max virtual memory areas vm.max_map_count")); maxMapCount.set(randomIntBetween(limit + 1, Integer.MAX_VALUE)); - BootstrapCheck.check(true, Collections.singletonList(check), "testMaxMapCountCheck"); + BootstrapCheck.check(true, false, Collections.singletonList(check), "testMaxMapCountCheck"); // nothing should happen if current vm.max_map_count is not // available maxMapCount.set(-1); - BootstrapCheck.check(true, Collections.singletonList(check), "testMaxMapCountCheck"); + BootstrapCheck.check(true, false, Collections.singletonList(check), "testMaxMapCountCheck"); } public void testMinMasterNodes() { @@ -353,7 +370,42 @@ public class BootstrapCheckTests extends ESTestCase { assertThat(check.check(), not(equalTo(isSet))); List defaultChecks = BootstrapCheck.checks(Settings.EMPTY); - expectThrows(RuntimeException.class, () -> BootstrapCheck.check(true, defaultChecks, "testMinMasterNodes")); + expectThrows(RuntimeException.class, () -> BootstrapCheck.check(true, false, defaultChecks, "testMinMasterNodes")); + } + + public void testIgnoringSystemChecks() { + BootstrapCheck.Check check = new BootstrapCheck.Check() { + @Override + public boolean check() { + return true; + } + + @Override + public String errorMessage() { + return "error"; + } + + @Override + public boolean isSystemCheck() { + return true; + } + }; + + final RuntimeException notIgnored = expectThrows( + RuntimeException.class, + () -> BootstrapCheck.check(true, false, Collections.singletonList(check), "testIgnoringSystemChecks")); + assertThat(notIgnored, hasToString(containsString("error"))); + + final ESLogger logger = mock(ESLogger.class); + + // nothing should happen if we ignore system checks + BootstrapCheck.check(true, true, Collections.singletonList(check), logger); + verify(logger).warn("error"); + reset(logger); + + // nothing should happen if we ignore all checks + BootstrapCheck.check(false, randomBoolean(), Collections.singletonList(check), logger); + verify(logger).warn("error"); } }