From f0d9177d8ef6293a55918320bedc26671b1cbe08 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Wed, 11 Jun 2025 16:25:21 +1000 Subject: [PATCH 1/3] Update tests for ElasticsearchNodeCommand The PR updates ElasticsearchNodeCommandTests so that they test the right behaviours after multi-project related changes: 1. Persisted cluster state is always written with multi-project enabled 2. The CLI tools are expected to run on the same node version, i.e. no BWC 3. Unknown (unregistered) customs can be read and write by the CLI tools without modification Resolves: ES-11328 --- .../gateway/PersistedClusterStateService.java | 2 +- .../ElasticsearchNodeCommandTests.java | 203 ++++++++++++++---- 2 files changed, 159 insertions(+), 46 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/gateway/PersistedClusterStateService.java b/server/src/main/java/org/elasticsearch/gateway/PersistedClusterStateService.java index 07e9cec009013..0681d2edd0dac 100644 --- a/server/src/main/java/org/elasticsearch/gateway/PersistedClusterStateService.java +++ b/server/src/main/java/org/elasticsearch/gateway/PersistedClusterStateService.java @@ -783,7 +783,7 @@ private static BytesReference uncompress(BytesReference bytesReference) throws I private static final ToXContent.Params FORMAT_PARAMS; static { - Map params = Maps.newMapWithExpectedSize(2); + Map params = Maps.newMapWithExpectedSize(4); params.put("binary", "true"); params.put(Metadata.CONTEXT_MODE_PARAM, Metadata.CONTEXT_MODE_GATEWAY); params.put(Metadata.DEDUPLICATED_MAPPINGS_PARAM, Boolean.TRUE.toString()); diff --git a/server/src/test/java/org/elasticsearch/cluster/coordination/ElasticsearchNodeCommandTests.java b/server/src/test/java/org/elasticsearch/cluster/coordination/ElasticsearchNodeCommandTests.java index 9a01f44fa067f..622aa9c82c55e 100644 --- a/server/src/test/java/org/elasticsearch/cluster/coordination/ElasticsearchNodeCommandTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/coordination/ElasticsearchNodeCommandTests.java @@ -9,23 +9,30 @@ package org.elasticsearch.cluster.coordination; +import org.elasticsearch.TransportVersion; import org.elasticsearch.cluster.ClusterModule; +import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.IndexGraveyard; import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.common.UUIDs; -import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.settings.ClusterSettings; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.core.Tuple; +import org.elasticsearch.env.Environment; +import org.elasticsearch.env.TestEnvironment; +import org.elasticsearch.gateway.PersistedClusterStateService; import org.elasticsearch.index.Index; import org.elasticsearch.indices.IndicesModule; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.TestClusterCustomMetadata; +import org.elasticsearch.test.TestProjectCustomMetadata; import org.elasticsearch.xcontent.NamedXContentRegistry; -import org.elasticsearch.xcontent.XContentBuilder; -import org.elasticsearch.xcontent.XContentParser; -import org.elasticsearch.xcontent.XContentParserConfiguration; -import org.elasticsearch.xcontent.json.JsonXContent; +import org.elasticsearch.xcontent.ParseField; import java.io.IOException; import java.nio.file.Path; +import java.util.EnumSet; import java.util.List; import java.util.function.Function; import java.util.stream.Stream; @@ -34,65 +41,100 @@ import static org.elasticsearch.cluster.metadata.DataStreamTestHelper.newInstance; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; -import static org.hamcrest.Matchers.not; public class ElasticsearchNodeCommandTests extends ESTestCase { - public void testLoadStateWithoutMissingCustoms() throws IOException { - runLoadStateTest(false, false); - } - public void testLoadStateWithoutMissingCustomsButPreserved() throws IOException { - runLoadStateTest(false, true); + runLoadStateTest(false); } public void testLoadStateWithMissingCustomsButPreserved() throws IOException { - runLoadStateTest(true, true); + runLoadStateTest(true); } - public void testLoadStateWithMissingCustomsAndNotPreserved() throws IOException { - runLoadStateTest(true, false); + @Override + public Settings buildEnvSettings(Settings settings) { + // we control the data path in the tests, so we don't need to set it here + return settings; } - private void runLoadStateTest(boolean hasMissingCustoms, boolean preserveUnknownCustoms) throws IOException { - final Metadata latestMetadata = randomMeta(); - final XContentBuilder builder = JsonXContent.contentBuilder(); - builder.startObject(); - Metadata.FORMAT.toXContent(builder, latestMetadata); - builder.endObject(); - - XContentParserConfiguration parserConfig = hasMissingCustoms - ? parserConfig().withRegistry(ElasticsearchNodeCommand.namedXContentRegistry) - : parserConfig(); - Metadata loadedMetadata; - try (XContentParser parser = createParser(parserConfig, JsonXContent.jsonXContent, BytesReference.bytes(builder))) { - loadedMetadata = Metadata.fromXContent(parser); - } - assertThat(loadedMetadata.clusterUUID(), not(equalTo("_na_"))); - assertThat(loadedMetadata.clusterUUID(), equalTo(latestMetadata.clusterUUID())); - assertThat(loadedMetadata.getProject().dataStreams(), equalTo(latestMetadata.getProject().dataStreams())); + private void runLoadStateTest(boolean hasMissingCustoms) throws IOException { + final var dataPath = createTempDir(); + final Settings settings = Settings.builder() + .putList(Environment.PATH_DATA_SETTING.getKey(), List.of(dataPath.toString())) + .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toAbsolutePath()) + .build(); - // make sure the index tombstones are the same too - if (hasMissingCustoms) { + try (var nodeEnvironment = newNodeEnvironment(settings)) { + final var persistedClusterStateServiceWithFullRegistry = new PersistedClusterStateService( + nodeEnvironment, + xContentRegistry(), + new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS), + () -> 0L, + () -> false + ); + + // 1. Simulating persisting cluster state by a running node + final long initialTerm = randomNonNegativeLong(); + final Metadata initialMetadata = randomMeta(hasMissingCustoms); + final ClusterState initialState = ClusterState.builder(ClusterState.EMPTY_STATE).metadata(initialMetadata).build(); + try (var writer = persistedClusterStateServiceWithFullRegistry.createWriter()) { + writer.writeFullStateAndCommit(initialTerm, initialState); + } + + // 2. Simulating loading the persisted cluster state by the CLI + final var persistedClusterStateServiceForNodeCommand = ElasticsearchNodeCommand.createPersistedClusterStateService( + Settings.EMPTY, + new Path[] { dataPath } + ); + final Tuple loadedTermAndClusterState = ElasticsearchNodeCommand.loadTermAndClusterState( + persistedClusterStateServiceForNodeCommand, + TestEnvironment.newEnvironment(settings) + ); + + assertThat(loadedTermAndClusterState.v1(), equalTo(initialTerm)); + final var loadedMetadata = loadedTermAndClusterState.v2().metadata(); assertNotNull(loadedMetadata.getProject().custom(IndexGraveyard.TYPE)); assertThat( loadedMetadata.getProject().custom(IndexGraveyard.TYPE), instanceOf(ElasticsearchNodeCommand.UnknownProjectCustom.class) ); + if (hasMissingCustoms) { + assertThat( + loadedMetadata.getProject().custom(MissingProjectCustomMetadata.TYPE), + instanceOf(ElasticsearchNodeCommand.UnknownProjectCustom.class) + ); + assertThat( + loadedMetadata.custom(MissingClusterCustomMetadata.TYPE), + instanceOf(ElasticsearchNodeCommand.UnknownClusterCustom.class) + ); + } else { + assertNull(loadedMetadata.getProject().custom(MissingProjectCustomMetadata.TYPE)); + assertNull(loadedMetadata.custom(MissingClusterCustomMetadata.TYPE)); + } - if (preserveUnknownCustoms) { - // check that we reserialize unknown metadata correctly again - final Path tempdir = createTempDir(); - Metadata.FORMAT.write(loadedMetadata, tempdir); - final Metadata reloadedMetadata = Metadata.FORMAT.loadLatestState(logger, xContentRegistry(), tempdir); - assertThat(reloadedMetadata.getProject().indexGraveyard(), equalTo(latestMetadata.getProject().indexGraveyard())); + final long newTerm = initialTerm + 1; + try (var writer = persistedClusterStateServiceForNodeCommand.createWriter()) { + writer.writeFullStateAndCommit(newTerm, ClusterState.builder(ClusterState.EMPTY_STATE).metadata(loadedMetadata).build()); + } + + // 3. Simulate node restart after updating on-disk state with the CLI tool + final var bestOnDiskState = persistedClusterStateServiceWithFullRegistry.loadBestOnDiskState(); + assertThat(bestOnDiskState.currentTerm, equalTo(newTerm)); + final Metadata reloadedMetadata = bestOnDiskState.metadata; + assertThat(reloadedMetadata.getProject().indexGraveyard(), equalTo(initialMetadata.getProject().indexGraveyard())); + if (hasMissingCustoms) { + assertThat( + reloadedMetadata.getProject().custom(MissingProjectCustomMetadata.TYPE), + equalTo(initialMetadata.getProject().custom(MissingProjectCustomMetadata.TYPE)) + ); + } else { + assertNull(reloadedMetadata.getProject().custom(MissingProjectCustomMetadata.TYPE)); } - } else { - assertThat(loadedMetadata.getProject().indexGraveyard(), equalTo(latestMetadata.getProject().indexGraveyard())); } } - private Metadata randomMeta() { + private Metadata randomMeta(boolean hasMissingCustoms) { Metadata.Builder mdBuilder = Metadata.builder(); mdBuilder.generateClusterUuidIfNeeded(); int numDelIndices = randomIntBetween(0, 5); @@ -109,15 +151,86 @@ private Metadata randomMeta() { } } mdBuilder.indexGraveyard(graveyard.build()); + if (hasMissingCustoms) { + mdBuilder.putCustom( + MissingProjectCustomMetadata.TYPE, + new MissingProjectCustomMetadata("test missing project custom metadata") + ); + mdBuilder.putCustom( + MissingClusterCustomMetadata.TYPE, + new MissingClusterCustomMetadata("test missing cluster custom metadata") + ); + } return mdBuilder.build(); } @Override protected NamedXContentRegistry xContentRegistry() { return new NamedXContentRegistry( - Stream.of(ClusterModule.getNamedXWriteables().stream(), IndicesModule.getNamedXContents().stream()) - .flatMap(Function.identity()) - .toList() + Stream.of( + ClusterModule.getNamedXWriteables().stream(), + IndicesModule.getNamedXContents().stream(), + Stream.of( + new NamedXContentRegistry.Entry( + Metadata.ProjectCustom.class, + new ParseField(MissingProjectCustomMetadata.TYPE), + parser -> MissingProjectCustomMetadata.fromXContent(MissingProjectCustomMetadata::new, parser) + ), + new NamedXContentRegistry.Entry( + Metadata.ClusterCustom.class, + new ParseField(MissingClusterCustomMetadata.TYPE), + parser -> MissingClusterCustomMetadata.fromXContent(MissingClusterCustomMetadata::new, parser) + ) + ) + ).flatMap(Function.identity()).toList() ); } + + private static class MissingProjectCustomMetadata extends TestProjectCustomMetadata { + + static final String TYPE = "missing_project_custom_metadata"; + + MissingProjectCustomMetadata(String data) { + super(data); + } + + @Override + public String getWriteableName() { + return TYPE; + } + + @Override + public TransportVersion getMinimalSupportedVersion() { + return TransportVersion.current(); + } + + @Override + public EnumSet context() { + return EnumSet.of(Metadata.XContentContext.GATEWAY); + } + } + + private static class MissingClusterCustomMetadata extends TestClusterCustomMetadata { + + static final String TYPE = "missing_cluster_custom_metadata"; + + MissingClusterCustomMetadata(String data) { + super(data); + } + + @Override + public String getWriteableName() { + return TYPE; + } + + @Override + public TransportVersion getMinimalSupportedVersion() { + return TransportVersion.current(); + } + + @Override + public EnumSet context() { + return EnumSet.of(Metadata.XContentContext.GATEWAY); + } + } } From 04bf1a11b9bf15c60d2915e6ad9d7b7d0f46fdd1 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Wed, 11 Jun 2025 16:42:36 +1000 Subject: [PATCH 2/3] tweak --- .../ElasticsearchNodeCommandTests.java | 43 +++++++++++-------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/cluster/coordination/ElasticsearchNodeCommandTests.java b/server/src/test/java/org/elasticsearch/cluster/coordination/ElasticsearchNodeCommandTests.java index 622aa9c82c55e..4fbb7c069b836 100644 --- a/server/src/test/java/org/elasticsearch/cluster/coordination/ElasticsearchNodeCommandTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/coordination/ElasticsearchNodeCommandTests.java @@ -41,6 +41,7 @@ import static org.elasticsearch.cluster.metadata.DataStreamTestHelper.newInstance; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.not; public class ElasticsearchNodeCommandTests extends ESTestCase { @@ -93,7 +94,11 @@ private void runLoadStateTest(boolean hasMissingCustoms) throws IOException { ); assertThat(loadedTermAndClusterState.v1(), equalTo(initialTerm)); + final var loadedMetadata = loadedTermAndClusterState.v2().metadata(); + assertThat(loadedMetadata.clusterUUID(), not(equalTo("_na_"))); + assertThat(loadedMetadata.clusterUUID(), equalTo(initialMetadata.clusterUUID())); + assertThat(loadedMetadata.getProject().dataStreams(), equalTo(initialMetadata.getProject().dataStreams())); assertNotNull(loadedMetadata.getProject().custom(IndexGraveyard.TYPE)); assertThat( loadedMetadata.getProject().custom(IndexGraveyard.TYPE), @@ -101,16 +106,16 @@ private void runLoadStateTest(boolean hasMissingCustoms) throws IOException { ); if (hasMissingCustoms) { assertThat( - loadedMetadata.getProject().custom(MissingProjectCustomMetadata.TYPE), + loadedMetadata.getProject().custom(TestMissingProjectCustomMetadata.TYPE), instanceOf(ElasticsearchNodeCommand.UnknownProjectCustom.class) ); assertThat( - loadedMetadata.custom(MissingClusterCustomMetadata.TYPE), + loadedMetadata.custom(TestMissingClusterCustomMetadata.TYPE), instanceOf(ElasticsearchNodeCommand.UnknownClusterCustom.class) ); } else { - assertNull(loadedMetadata.getProject().custom(MissingProjectCustomMetadata.TYPE)); - assertNull(loadedMetadata.custom(MissingClusterCustomMetadata.TYPE)); + assertNull(loadedMetadata.getProject().custom(TestMissingProjectCustomMetadata.TYPE)); + assertNull(loadedMetadata.custom(TestMissingClusterCustomMetadata.TYPE)); } final long newTerm = initialTerm + 1; @@ -125,11 +130,11 @@ private void runLoadStateTest(boolean hasMissingCustoms) throws IOException { assertThat(reloadedMetadata.getProject().indexGraveyard(), equalTo(initialMetadata.getProject().indexGraveyard())); if (hasMissingCustoms) { assertThat( - reloadedMetadata.getProject().custom(MissingProjectCustomMetadata.TYPE), - equalTo(initialMetadata.getProject().custom(MissingProjectCustomMetadata.TYPE)) + reloadedMetadata.getProject().custom(TestMissingProjectCustomMetadata.TYPE), + equalTo(initialMetadata.getProject().custom(TestMissingProjectCustomMetadata.TYPE)) ); } else { - assertNull(reloadedMetadata.getProject().custom(MissingProjectCustomMetadata.TYPE)); + assertNull(reloadedMetadata.getProject().custom(TestMissingProjectCustomMetadata.TYPE)); } } } @@ -153,12 +158,12 @@ private Metadata randomMeta(boolean hasMissingCustoms) { mdBuilder.indexGraveyard(graveyard.build()); if (hasMissingCustoms) { mdBuilder.putCustom( - MissingProjectCustomMetadata.TYPE, - new MissingProjectCustomMetadata("test missing project custom metadata") + TestMissingProjectCustomMetadata.TYPE, + new TestMissingProjectCustomMetadata("test missing project custom metadata") ); mdBuilder.putCustom( - MissingClusterCustomMetadata.TYPE, - new MissingClusterCustomMetadata("test missing cluster custom metadata") + TestMissingClusterCustomMetadata.TYPE, + new TestMissingClusterCustomMetadata("test missing cluster custom metadata") ); } return mdBuilder.build(); @@ -173,24 +178,24 @@ protected NamedXContentRegistry xContentRegistry() { Stream.of( new NamedXContentRegistry.Entry( Metadata.ProjectCustom.class, - new ParseField(MissingProjectCustomMetadata.TYPE), - parser -> MissingProjectCustomMetadata.fromXContent(MissingProjectCustomMetadata::new, parser) + new ParseField(TestMissingProjectCustomMetadata.TYPE), + parser -> TestMissingProjectCustomMetadata.fromXContent(TestMissingProjectCustomMetadata::new, parser) ), new NamedXContentRegistry.Entry( Metadata.ClusterCustom.class, - new ParseField(MissingClusterCustomMetadata.TYPE), - parser -> MissingClusterCustomMetadata.fromXContent(MissingClusterCustomMetadata::new, parser) + new ParseField(TestMissingClusterCustomMetadata.TYPE), + parser -> TestMissingClusterCustomMetadata.fromXContent(TestMissingClusterCustomMetadata::new, parser) ) ) ).flatMap(Function.identity()).toList() ); } - private static class MissingProjectCustomMetadata extends TestProjectCustomMetadata { + private static class TestMissingProjectCustomMetadata extends TestProjectCustomMetadata { static final String TYPE = "missing_project_custom_metadata"; - MissingProjectCustomMetadata(String data) { + TestMissingProjectCustomMetadata(String data) { super(data); } @@ -210,11 +215,11 @@ public EnumSet context() { } } - private static class MissingClusterCustomMetadata extends TestClusterCustomMetadata { + private static class TestMissingClusterCustomMetadata extends TestClusterCustomMetadata { static final String TYPE = "missing_cluster_custom_metadata"; - MissingClusterCustomMetadata(String data) { + TestMissingClusterCustomMetadata(String data) { super(data); } From 691507a121d0b610148695ecbb4437591b834d4a Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Fri, 13 Jun 2025 13:25:09 +1000 Subject: [PATCH 3/3] add comments --- .../cluster/coordination/ElasticsearchNodeCommandTests.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/server/src/test/java/org/elasticsearch/cluster/coordination/ElasticsearchNodeCommandTests.java b/server/src/test/java/org/elasticsearch/cluster/coordination/ElasticsearchNodeCommandTests.java index 42fd9c418b71b..c2cf7adacf01d 100644 --- a/server/src/test/java/org/elasticsearch/cluster/coordination/ElasticsearchNodeCommandTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/coordination/ElasticsearchNodeCommandTests.java @@ -196,6 +196,9 @@ protected NamedXContentRegistry xContentRegistry() { ); } + // "Missing" really means _not_ registered in ElasticsearchNodeCommand's xContentRegistry so that it will be read and written + // as a generic unknown project custom, see also ElasticsearchNodeCommand.UnknownProjectCustom. Note it is registered in the + // full xContentRegistry used by the Elasticsearch node. That is how it got written in the first place. private static class TestMissingProjectCustomMetadata extends TestProjectCustomMetadata { static final String TYPE = "missing_project_custom_metadata"; @@ -220,6 +223,7 @@ public EnumSet context() { } } + // Similar "Missing" custom but in the cluster scope. See also the comment above for TestMissingProjectCustomMetadata. private static class TestMissingClusterCustomMetadata extends TestClusterCustomMetadata { static final String TYPE = "missing_cluster_custom_metadata";