Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -783,7 +783,7 @@ private static BytesReference uncompress(BytesReference bytesReference) throws I
private static final ToXContent.Params FORMAT_PARAMS;

static {
Map<String, String> params = Maps.newMapWithExpectedSize(2);
Map<String, String> params = Maps.newMapWithExpectedSize(4);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 tho unrelated to the test changes

params.put("binary", "true");
params.put(Metadata.CONTEXT_MODE_PARAM, Metadata.CONTEXT_MODE_GATEWAY);
params.put(Metadata.DEDUPLICATED_MAPPINGS_PARAM, Boolean.TRUE.toString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -38,61 +45,101 @@

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);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is no longer correct after MP changes because it serializes metadata without multi-project=true and that is what PersistedClusterStateService does. The CLI tools are never run across versions so that the test needs a serialized state with up-to-date format (by PersistedClusterStateService).

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);
}
Comment on lines -70 to -73
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This round-trip does not seem to directly test ElasticsearchNodeCommand at all. It is replaced directly calling ElasticsearchNodeCommand#loadTermAndClusterState.

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<Long, ClusterState> loadedTermAndClusterState = ElasticsearchNodeCommand.loadTermAndClusterState(
persistedClusterStateServiceForNodeCommand,
TestEnvironment.newEnvironment(settings)
);

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),
instanceOf(ElasticsearchNodeCommand.UnknownProjectCustom.class)
);
if (hasMissingCustoms) {
assertThat(
loadedMetadata.getProject().custom(TestMissingProjectCustomMetadata.TYPE),
instanceOf(ElasticsearchNodeCommand.UnknownProjectCustom.class)
);
assertThat(
loadedMetadata.custom(TestMissingClusterCustomMetadata.TYPE),
instanceOf(ElasticsearchNodeCommand.UnknownClusterCustom.class)
);
} else {
assertNull(loadedMetadata.getProject().custom(TestMissingProjectCustomMetadata.TYPE));
assertNull(loadedMetadata.custom(TestMissingClusterCustomMetadata.TYPE));
}

final long newTerm = initialTerm + 1;
try (var writer = persistedClusterStateServiceForNodeCommand.createWriter()) {
writer.writeFullStateAndCommit(newTerm, ClusterState.builder(ClusterState.EMPTY_STATE).metadata(loadedMetadata).build());
}

if (preserveUnknownCustoms) {
// check that we reserialize unknown metadata correctly again
final Path tempdir = createTempDir();
Metadata.FORMAT.write(loadedMetadata, tempdir);
Comment on lines -86 to -89
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The node CLI always perserve unknown customs as long as they are either ClusterCustom, ProjectCustom or Condition. Therefore the PR removes test variations based on the preserveUnknownCustoms boolean. The Metadata.FORMAT.write method has the problem of not using multi-project=true when serializing. It is replaced by using the PersistedClusterStateService created as part of the node CLI.

final Metadata reloadedMetadata = Metadata.FORMAT.loadLatestState(logger, xContentRegistry(), tempdir);
assertThat(reloadedMetadata.getProject().indexGraveyard(), equalTo(latestMetadata.getProject().indexGraveyard()));
// 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(TestMissingProjectCustomMetadata.TYPE),
equalTo(initialMetadata.getProject().custom(TestMissingProjectCustomMetadata.TYPE))
);
} else {
assertNull(reloadedMetadata.getProject().custom(TestMissingProjectCustomMetadata.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);
Expand All @@ -109,15 +156,86 @@ private Metadata randomMeta() {
}
}
mdBuilder.indexGraveyard(graveyard.build());
if (hasMissingCustoms) {
mdBuilder.putCustom(
TestMissingProjectCustomMetadata.TYPE,
new TestMissingProjectCustomMetadata("test missing project custom metadata")
);
mdBuilder.putCustom(
TestMissingClusterCustomMetadata.TYPE,
new TestMissingClusterCustomMetadata("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(TestMissingProjectCustomMetadata.TYPE),
parser -> TestMissingProjectCustomMetadata.fromXContent(TestMissingProjectCustomMetadata::new, parser)
),
new NamedXContentRegistry.Entry(
Metadata.ClusterCustom.class,
new ParseField(TestMissingClusterCustomMetadata.TYPE),
parser -> TestMissingClusterCustomMetadata.fromXContent(TestMissingClusterCustomMetadata::new, parser)
)
)
).flatMap(Function.identity()).toList()
);
}

private static class TestMissingProjectCustomMetadata extends TestProjectCustomMetadata {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually simulating unknown (unregistered) customs.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deserves a comment in the code to that effect - the "missing" is a little confusing cos they're not missing, they're right here, the point is that they're not registered. Except they are registered in some registries, just not the one that the ElasticsearchNodeCommand uses.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah see 691507a


static final String TYPE = "missing_project_custom_metadata";

TestMissingProjectCustomMetadata(String data) {
super(data);
}

@Override
public String getWriteableName() {
return TYPE;
}

@Override
public TransportVersion getMinimalSupportedVersion() {
return TransportVersion.current();
}

@Override
public EnumSet<Metadata.XContentContext> context() {
return EnumSet.of(Metadata.XContentContext.GATEWAY);
}
}

private static class TestMissingClusterCustomMetadata extends TestClusterCustomMetadata {

static final String TYPE = "missing_cluster_custom_metadata";

TestMissingClusterCustomMetadata(String data) {
super(data);
}

@Override
public String getWriteableName() {
return TYPE;
}

@Override
public TransportVersion getMinimalSupportedVersion() {
return TransportVersion.current();
}

@Override
public EnumSet<Metadata.XContentContext> context() {
return EnumSet.of(Metadata.XContentContext.GATEWAY);
}
}
}