From 1207b53a24715bc2bbbc3586bdccae20966e93c7 Mon Sep 17 00:00:00 2001 From: Xue Zhou Date: Tue, 21 Feb 2023 15:58:57 -0800 Subject: [PATCH 1/6] Add bootstrap check to avoid search node has multiple data path Signed-off-by: Xue Zhou --- .../opensearch/bootstrap/BootstrapChecks.java | 25 +++++++++++++++++++ .../bootstrap/BootstrapChecksTests.java | 23 +++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/server/src/main/java/org/opensearch/bootstrap/BootstrapChecks.java b/server/src/main/java/org/opensearch/bootstrap/BootstrapChecks.java index 5d595dc1abedf..b82a432013468 100644 --- a/server/src/main/java/org/opensearch/bootstrap/BootstrapChecks.java +++ b/server/src/main/java/org/opensearch/bootstrap/BootstrapChecks.java @@ -38,15 +38,18 @@ import org.apache.lucene.util.Constants; import org.opensearch.bootstrap.jvm.DenyJvmVersionsParser; import org.opensearch.cluster.coordination.ClusterBootstrapService; +import org.opensearch.cluster.node.DiscoveryNodeRole; import org.opensearch.common.SuppressForbidden; import org.opensearch.common.io.PathUtils; import org.opensearch.common.settings.Setting; import org.opensearch.common.transport.BoundTransportAddress; import org.opensearch.common.transport.TransportAddress; import org.opensearch.discovery.DiscoveryModule; +import org.opensearch.env.Environment; import org.opensearch.index.IndexModule; import org.opensearch.monitor.jvm.JvmInfo; import org.opensearch.monitor.process.ProcessProbe; +import org.opensearch.node.NodeRoleSettings; import org.opensearch.node.NodeValidationException; import java.io.BufferedReader; @@ -228,6 +231,7 @@ static List checks() { checks.add(new JavaVersionCheck()); checks.add(new AllPermissionCheck()); checks.add(new DiscoveryConfiguredCheck()); + checks.add(new MultipleDataPathCheck()); return Collections.unmodifiableList(checks); } @@ -751,4 +755,25 @@ public BootstrapCheckResult check(BootstrapContext context) { ); } } + + /** + * Bootstrap check that if a search node contains multiple data paths + */ + static class MultipleDataPathCheck implements BootstrapCheck { + + @Override + public BootstrapCheckResult check(BootstrapContext context) { + if (NodeRoleSettings.NODE_ROLES_SETTING.get(context.settings()).contains(DiscoveryNodeRole.SEARCH_ROLE) + && Environment.PATH_DATA_SETTING.get(context.settings()).size() > 1) { + return BootstrapCheckResult.failure("Having multiple data paths in the search node is not allowed"); + } + return BootstrapCheckResult.success(); + } + + @Override + public final boolean alwaysEnforce() { + return true; + } + + } } diff --git a/server/src/test/java/org/opensearch/bootstrap/BootstrapChecksTests.java b/server/src/test/java/org/opensearch/bootstrap/BootstrapChecksTests.java index 8edb204ac0402..d310bc438c0d2 100644 --- a/server/src/test/java/org/opensearch/bootstrap/BootstrapChecksTests.java +++ b/server/src/test/java/org/opensearch/bootstrap/BootstrapChecksTests.java @@ -36,19 +36,24 @@ import org.apache.lucene.util.Constants; import org.opensearch.cluster.coordination.ClusterBootstrapService; import org.opensearch.cluster.metadata.Metadata; +import org.opensearch.cluster.node.DiscoveryNodeRole; import org.opensearch.common.CheckedConsumer; +import org.opensearch.common.io.PathUtils; import org.opensearch.common.settings.Settings; import org.opensearch.common.transport.BoundTransportAddress; import org.opensearch.common.transport.TransportAddress; import org.opensearch.discovery.DiscoveryModule; import org.opensearch.discovery.SettingsBasedSeedHostsProvider; +import org.opensearch.env.Environment; import org.opensearch.monitor.jvm.JvmInfo; +import org.opensearch.node.NodeRoleSettings; import org.opensearch.node.NodeValidationException; import org.opensearch.test.AbstractBootstrapCheckTestCase; import org.hamcrest.Matcher; import java.lang.Runtime.Version; import java.net.InetAddress; +import java.nio.file.Path; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -774,4 +779,22 @@ Version getVersion() { version.set(Runtime.version()); BootstrapChecks.check(emptyContext, true, Collections.singletonList(check)); } + + public void testMultipleDataPathCheck() throws NodeValidationException { + Path path = PathUtils.get(createTempDir().toString()); + String[] paths = new String[] { path.resolve("a").toString(), path.resolve("b").toString() }; + + final BootstrapContext context = createTestContext( + Settings.builder() + .putList(NodeRoleSettings.NODE_ROLES_SETTING.getKey(), Collections.singletonList(DiscoveryNodeRole.SEARCH_ROLE.roleName())) + .putList(Environment.PATH_DATA_SETTING.getKey(), paths) + .build(), + Metadata.EMPTY_METADATA + ); + final List checks = Collections.singletonList(new BootstrapChecks.MultipleDataPathCheck()); + final NodeValidationException e = expectThrows(NodeValidationException.class, () -> BootstrapChecks.check(context, true, checks)); + assertThat(e.getMessage(), containsString("Having multiple data paths in the search role is not allowed")); + // Validate the check passes under default setting. + BootstrapChecks.check(emptyContext, true, checks); + } } From d1a7b870de2eb075f8608e6400bf52f2db7696e1 Mon Sep 17 00:00:00 2001 From: Xue Zhou Date: Tue, 21 Feb 2023 16:26:21 -0800 Subject: [PATCH 2/6] Add changelog Signed-off-by: Xue Zhou --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e5c60bdc99db9..4732fe0e5c8cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -101,6 +101,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Fix timeout error when adding a document to an index with extension running ([#6275](https://github.com/opensearch-project/OpenSearch/pull/6275)) - Handle translog upload during primary relocation for remote-backed indexes ([#5804](https://github.com/opensearch-project/OpenSearch/pull/5804)) - Batch translog sync/upload per x ms for remote-backed indexes ([#5854](https://github.com/opensearch-project/OpenSearch/pull/5854)) +- Add bootstrap check to avoid search node has multiple data paths ([#6427](https://github.com/opensearch-project/OpenSearch/pull/6427)) ### Dependencies - Update nebula-publishing-plugin to 19.2.0 ([#5704](https://github.com/opensearch-project/OpenSearch/pull/5704)) From 6a3cb90547393d46be369add26f524ead17408d3 Mon Sep 17 00:00:00 2001 From: Xue Zhou Date: Wed, 22 Feb 2023 22:16:52 -0800 Subject: [PATCH 3/6] Modify comments Signed-off-by: Xue Zhou --- CHANGELOG.md | 2 +- .../opensearch/bootstrap/BootstrapChecks.java | 2 +- .../bootstrap/BootstrapChecksTests.java | 34 +++++++++++++++++-- 3 files changed, 33 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4732fe0e5c8cb..82729606e6262 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -101,7 +101,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Fix timeout error when adding a document to an index with extension running ([#6275](https://github.com/opensearch-project/OpenSearch/pull/6275)) - Handle translog upload during primary relocation for remote-backed indexes ([#5804](https://github.com/opensearch-project/OpenSearch/pull/5804)) - Batch translog sync/upload per x ms for remote-backed indexes ([#5854](https://github.com/opensearch-project/OpenSearch/pull/5854)) -- Add bootstrap check to avoid search node has multiple data paths ([#6427](https://github.com/opensearch-project/OpenSearch/pull/6427)) +- Disallow multiple data paths for search nodes ([#6427](https://github.com/opensearch-project/OpenSearch/pull/6427)) ### Dependencies - Update nebula-publishing-plugin to 19.2.0 ([#5704](https://github.com/opensearch-project/OpenSearch/pull/5704)) diff --git a/server/src/main/java/org/opensearch/bootstrap/BootstrapChecks.java b/server/src/main/java/org/opensearch/bootstrap/BootstrapChecks.java index b82a432013468..8df1667a01910 100644 --- a/server/src/main/java/org/opensearch/bootstrap/BootstrapChecks.java +++ b/server/src/main/java/org/opensearch/bootstrap/BootstrapChecks.java @@ -759,7 +759,7 @@ public BootstrapCheckResult check(BootstrapContext context) { /** * Bootstrap check that if a search node contains multiple data paths */ - static class MultipleDataPathCheck implements BootstrapCheck { + static class MultipleDataPathCheck implements BootstrapCheck { @Override public BootstrapCheckResult check(BootstrapContext context) { diff --git a/server/src/test/java/org/opensearch/bootstrap/BootstrapChecksTests.java b/server/src/test/java/org/opensearch/bootstrap/BootstrapChecksTests.java index d310bc438c0d2..a02b00d090d06 100644 --- a/server/src/test/java/org/opensearch/bootstrap/BootstrapChecksTests.java +++ b/server/src/test/java/org/opensearch/bootstrap/BootstrapChecksTests.java @@ -780,7 +780,7 @@ Version getVersion() { BootstrapChecks.check(emptyContext, true, Collections.singletonList(check)); } - public void testMultipleDataPathCheck() throws NodeValidationException { + public void testMultipleDataPathsForSearchNodeCheck() throws NodeValidationException { Path path = PathUtils.get(createTempDir().toString()); String[] paths = new String[] { path.resolve("a").toString(), path.resolve("b").toString() }; @@ -793,8 +793,36 @@ public void testMultipleDataPathCheck() throws NodeValidationException { ); final List checks = Collections.singletonList(new BootstrapChecks.MultipleDataPathCheck()); final NodeValidationException e = expectThrows(NodeValidationException.class, () -> BootstrapChecks.check(context, true, checks)); - assertThat(e.getMessage(), containsString("Having multiple data paths in the search role is not allowed")); - // Validate the check passes under default setting. + assertThat(e.getMessage(), containsString("Having multiple data paths in the search node is not allowed")); + } + + public void testMultipleDataPathsForDataNodeCheck() throws NodeValidationException { + Path path = PathUtils.get(createTempDir().toString()); + String[] paths = new String[] { path.resolve("a").toString(), path.resolve("b").toString() }; + + final BootstrapContext context = createTestContext( + Settings.builder() + .putList(NodeRoleSettings.NODE_ROLES_SETTING.getKey(), Collections.singletonList(DiscoveryNodeRole.DATA_ROLE.roleName())) + .putList(Environment.PATH_DATA_SETTING.getKey(), paths) + .build(), + Metadata.EMPTY_METADATA + ); + final List checks = Collections.singletonList(new BootstrapChecks.MultipleDataPathCheck()); + BootstrapChecks.check(emptyContext, true, checks); + } + + public void testSingleDataPathForSearchNodeCheck() throws NodeValidationException { + Path path = PathUtils.get(createTempDir().toString()); + String[] paths = new String[] { path.resolve("a").toString() }; + + final BootstrapContext context = createTestContext( + Settings.builder() + .putList(NodeRoleSettings.NODE_ROLES_SETTING.getKey(), Collections.singletonList(DiscoveryNodeRole.SEARCH_ROLE.roleName())) + .putList(Environment.PATH_DATA_SETTING.getKey(), paths) + .build(), + Metadata.EMPTY_METADATA + ); + final List checks = Collections.singletonList(new BootstrapChecks.MultipleDataPathCheck()); BootstrapChecks.check(emptyContext, true, checks); } } From 411e7a52e66af5d803a602d100faaef0d8bd63a6 Mon Sep 17 00:00:00 2001 From: Xue Zhou Date: Wed, 22 Feb 2023 23:04:06 -0800 Subject: [PATCH 4/6] fix spotless check Signed-off-by: Xue Zhou --- .../src/main/java/org/opensearch/bootstrap/BootstrapChecks.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/bootstrap/BootstrapChecks.java b/server/src/main/java/org/opensearch/bootstrap/BootstrapChecks.java index 8df1667a01910..b82a432013468 100644 --- a/server/src/main/java/org/opensearch/bootstrap/BootstrapChecks.java +++ b/server/src/main/java/org/opensearch/bootstrap/BootstrapChecks.java @@ -759,7 +759,7 @@ public BootstrapCheckResult check(BootstrapContext context) { /** * Bootstrap check that if a search node contains multiple data paths */ - static class MultipleDataPathCheck implements BootstrapCheck { + static class MultipleDataPathCheck implements BootstrapCheck { @Override public BootstrapCheckResult check(BootstrapContext context) { From ec4a3d6135ab5f05ddf686399c04fc11eceab917 Mon Sep 17 00:00:00 2001 From: Xue Zhou Date: Thu, 23 Feb 2023 12:43:12 -0800 Subject: [PATCH 5/6] Modify comments Signed-off-by: Xue Zhou --- .../src/main/java/org/opensearch/bootstrap/BootstrapChecks.java | 2 +- .../java/org/opensearch/bootstrap/BootstrapChecksTests.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/opensearch/bootstrap/BootstrapChecks.java b/server/src/main/java/org/opensearch/bootstrap/BootstrapChecks.java index b82a432013468..c27c149947444 100644 --- a/server/src/main/java/org/opensearch/bootstrap/BootstrapChecks.java +++ b/server/src/main/java/org/opensearch/bootstrap/BootstrapChecks.java @@ -765,7 +765,7 @@ static class MultipleDataPathCheck implements BootstrapCheck { public BootstrapCheckResult check(BootstrapContext context) { if (NodeRoleSettings.NODE_ROLES_SETTING.get(context.settings()).contains(DiscoveryNodeRole.SEARCH_ROLE) && Environment.PATH_DATA_SETTING.get(context.settings()).size() > 1) { - return BootstrapCheckResult.failure("Having multiple data paths in the search node is not allowed"); + return BootstrapCheckResult.failure("Multiple data paths are not allowed for search nodes"); } return BootstrapCheckResult.success(); } diff --git a/server/src/test/java/org/opensearch/bootstrap/BootstrapChecksTests.java b/server/src/test/java/org/opensearch/bootstrap/BootstrapChecksTests.java index a02b00d090d06..b0b030200a110 100644 --- a/server/src/test/java/org/opensearch/bootstrap/BootstrapChecksTests.java +++ b/server/src/test/java/org/opensearch/bootstrap/BootstrapChecksTests.java @@ -793,7 +793,7 @@ public void testMultipleDataPathsForSearchNodeCheck() throws NodeValidationExcep ); final List checks = Collections.singletonList(new BootstrapChecks.MultipleDataPathCheck()); final NodeValidationException e = expectThrows(NodeValidationException.class, () -> BootstrapChecks.check(context, true, checks)); - assertThat(e.getMessage(), containsString("Having multiple data paths in the search node is not allowed")); + assertThat(e.getMessage(), containsString("Multiple data paths are not allowed for search nodes")); } public void testMultipleDataPathsForDataNodeCheck() throws NodeValidationException { From c9ff4d18785a47297d71a901c3f56f9a4753e17f Mon Sep 17 00:00:00 2001 From: Xue Zhou Date: Thu, 23 Feb 2023 13:44:12 -0800 Subject: [PATCH 6/6] Remove duplication Signed-off-by: Xue Zhou --- .../bootstrap/BootstrapChecksTests.java | 31 +++++++------------ 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/server/src/test/java/org/opensearch/bootstrap/BootstrapChecksTests.java b/server/src/test/java/org/opensearch/bootstrap/BootstrapChecksTests.java index b0b030200a110..15aacd25b30b1 100644 --- a/server/src/test/java/org/opensearch/bootstrap/BootstrapChecksTests.java +++ b/server/src/test/java/org/opensearch/bootstrap/BootstrapChecksTests.java @@ -780,19 +780,14 @@ Version getVersion() { BootstrapChecks.check(emptyContext, true, Collections.singletonList(check)); } - public void testMultipleDataPathsForSearchNodeCheck() throws NodeValidationException { + public void testMultipleDataPathsForSearchNodeCheck() { Path path = PathUtils.get(createTempDir().toString()); String[] paths = new String[] { path.resolve("a").toString(), path.resolve("b").toString() }; - final BootstrapContext context = createTestContext( - Settings.builder() - .putList(NodeRoleSettings.NODE_ROLES_SETTING.getKey(), Collections.singletonList(DiscoveryNodeRole.SEARCH_ROLE.roleName())) - .putList(Environment.PATH_DATA_SETTING.getKey(), paths) - .build(), - Metadata.EMPTY_METADATA + final NodeValidationException e = expectThrows( + NodeValidationException.class, + () -> performDataPathsCheck(paths, DiscoveryNodeRole.SEARCH_ROLE.roleName()) ); - final List checks = Collections.singletonList(new BootstrapChecks.MultipleDataPathCheck()); - final NodeValidationException e = expectThrows(NodeValidationException.class, () -> BootstrapChecks.check(context, true, checks)); assertThat(e.getMessage(), containsString("Multiple data paths are not allowed for search nodes")); } @@ -800,29 +795,25 @@ public void testMultipleDataPathsForDataNodeCheck() throws NodeValidationExcepti Path path = PathUtils.get(createTempDir().toString()); String[] paths = new String[] { path.resolve("a").toString(), path.resolve("b").toString() }; - final BootstrapContext context = createTestContext( - Settings.builder() - .putList(NodeRoleSettings.NODE_ROLES_SETTING.getKey(), Collections.singletonList(DiscoveryNodeRole.DATA_ROLE.roleName())) - .putList(Environment.PATH_DATA_SETTING.getKey(), paths) - .build(), - Metadata.EMPTY_METADATA - ); - final List checks = Collections.singletonList(new BootstrapChecks.MultipleDataPathCheck()); - BootstrapChecks.check(emptyContext, true, checks); + performDataPathsCheck(paths, DiscoveryNodeRole.DATA_ROLE.roleName()); } public void testSingleDataPathForSearchNodeCheck() throws NodeValidationException { Path path = PathUtils.get(createTempDir().toString()); String[] paths = new String[] { path.resolve("a").toString() }; + performDataPathsCheck(paths, DiscoveryNodeRole.SEARCH_ROLE.roleName()); + } + + private void performDataPathsCheck(String[] paths, String roleName) throws NodeValidationException { final BootstrapContext context = createTestContext( Settings.builder() - .putList(NodeRoleSettings.NODE_ROLES_SETTING.getKey(), Collections.singletonList(DiscoveryNodeRole.SEARCH_ROLE.roleName())) + .putList(NodeRoleSettings.NODE_ROLES_SETTING.getKey(), Collections.singletonList(roleName)) .putList(Environment.PATH_DATA_SETTING.getKey(), paths) .build(), Metadata.EMPTY_METADATA ); final List checks = Collections.singletonList(new BootstrapChecks.MultipleDataPathCheck()); - BootstrapChecks.check(emptyContext, true, checks); + BootstrapChecks.check(context, true, checks); } }