Skip to content

Commit

Permalink
Continue to evaulate / migrate after failures (opensearch-project#1037)
Browse files Browse the repository at this point in the history 
Updates the behavoir of the metadata processing for templates and
indexes to collect all the results and then return failure details after
all items have been processed.

Signed-off-by: Peter Nied <[email protected]>
  • Loading branch information
@peternied
peternied authored Oct 9, 2024
1 parent 376180d commit 7307579
Show file tree
Hide file tree
Showing 18 changed files with 378 additions and 113 deletions.
87 changes: 69 additions & 18 deletions MetadataMigration/src/main/java/org/opensearch/migrations/cli/Items.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import java.util.List;
import java.util.stream.Collectors;

import org.opensearch.migrations.metadata.CreationResult;

import lombok.Builder;
import lombok.Data;
import lombok.NonNull;
Expand All @@ -16,13 +18,14 @@ public class Items {
static final String NONE_FOUND_MARKER = "<NONE FOUND>";
private final boolean dryRun;
@NonNull
private final List<String> indexTemplates;
private final List<CreationResult> indexTemplates;
@NonNull
private final List<String> componentTemplates;
private final List<CreationResult> componentTemplates;
@NonNull
private final List<String> indexes;
private final List<CreationResult> indexes;
@NonNull
private final List<String> aliases;
private final List<CreationResult> aliases;
private final String failureMessage;

public String asCliOutput() {
var sb = new StringBuilder();
Expand All @@ -31,25 +34,73 @@ public String asCliOutput() {
} else {
sb.append("Migrated Items:" + System.lineSeparator());
}
sb.append(Format.indentToLevel(1) + "Index Templates:" + System.lineSeparator());
sb.append(Format.indentToLevel(2) + getPrintableList(getIndexTemplates()) + System.lineSeparator());
sb.append(System.lineSeparator());
sb.append(Format.indentToLevel(1) + "Component Templates:" + System.lineSeparator());
sb.append(Format.indentToLevel(2) +getPrintableList(getComponentTemplates()) + System.lineSeparator());
sb.append(System.lineSeparator());
sb.append(Format.indentToLevel(1) + "Indexes:" + System.lineSeparator());
sb.append(Format.indentToLevel(2) + getPrintableList(getIndexes()) + System.lineSeparator());
sb.append(System.lineSeparator());
sb.append(Format.indentToLevel(1) + "Aliases:" + System.lineSeparator());
sb.append(Format.indentToLevel(2) +getPrintableList(getAliases()) + System.lineSeparator());
appendSection(sb, "Index Templates", getIndexTemplates());
appendSection(sb, "Component Templates", getComponentTemplates());
appendSection(sb, "Indexes", getIndexes());
appendSection(sb, "Aliases", getAliases());

return sb.toString();
}

private void appendSection(StringBuilder sb, String sectionTitle, List<CreationResult> items) {
sb.append(Format.indentToLevel(1))
.append(sectionTitle)
.append(":")
.append(System.lineSeparator());

if (items.isEmpty()) {
sb.append(Format.indentToLevel(2))
.append(NONE_FOUND_MARKER)
.append(System.lineSeparator());
} else {
appendItems(sb, items);
appendFailures(sb, items);
}
sb.append(System.lineSeparator());
}

private void appendItems(StringBuilder sb, List<CreationResult> items) {
var successfulItems = items.stream()
.filter(r -> r.wasSuccessful())
.map(r -> r.getName())
.collect(Collectors.toList());

if (!successfulItems.isEmpty()) {
sb.append(Format.indentToLevel(2))
.append(getPrintableList(successfulItems))
.append(System.lineSeparator());
}
}

private void appendFailures(StringBuilder sb, List<CreationResult> items) {
var failures = items.stream()
.filter(r -> !r.wasSuccessful())
.map(this::failureMessage)
.sorted()
.collect(Collectors.toList());
if (!failures.isEmpty()) {
failures.forEach(failure -> sb.append(Format.indentToLevel(2))
.append(failure)
.append(System.lineSeparator()));
}
}

private String failureMessage(CreationResult result){
var sb = new StringBuilder()
.append(result.getFailureType().isFatal() ? "ERROR" : "WARN")
.append(" - ")
.append(result.getName())
.append(" ")
.append(result.getFailureType().getMessage());

if (result.getFailureType().isFatal()) {
sb.append(": " + result.getException().getMessage());
}

return sb.toString();
}

private String getPrintableList(List<String> list) {
if (list == null || list.isEmpty()) {
return NONE_FOUND_MARKER;
}
return list.stream().sorted().collect(Collectors.joining(", "));
}
}
17 changes: 13 additions & 4 deletions ...dataMigration/src/main/java/org/opensearch/migrations/commands/MigratorEvaluatorBase.java
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.opensearch.migrations.commands;

import java.util.ArrayList;
import java.util.List;

import org.opensearch.migrations.MigrateOrEvaluateArgs;
import org.opensearch.migrations.MigrationMode;
Expand All @@ -13,6 +14,7 @@
import org.opensearch.migrations.cli.Clusters;
import org.opensearch.migrations.cli.Items;
import org.opensearch.migrations.cluster.ClusterProviderRegistry;
import org.opensearch.migrations.metadata.CreationResult;
import org.opensearch.migrations.metadata.GlobalMetadataCreatorResults;
import org.opensearch.migrations.metadata.tracing.RootMetadataMigrationContext;

Expand Down Expand Up @@ -58,15 +60,22 @@ protected Items migrateAllItems(MigrationMode migrationMode, Clusters clusters,
items.dryRun(migrationMode.equals(MigrationMode.SIMULATE));
var metadataResults = migrateGlobalMetadata(migrationMode, clusters, transformer, context);

var indexTemplates = new ArrayList<String>();
var indexTemplates = new ArrayList<CreationResult>();
indexTemplates.addAll(metadataResults.getLegacyTemplates());
indexTemplates.addAll(metadataResults.getIndexTemplates());
items.indexTemplates(indexTemplates);
items.componentTemplates(metadataResults.getComponentTemplates());

var indexResults = migrateIndices(migrationMode, clusters, transformer, context);
items.indexes(indexResults.getIndexNames());
items.aliases(indexResults.getAliases());
if (metadataResults.fatalIssueCount() == 0) {
var indexResults = migrateIndices(migrationMode, clusters, transformer, context);
items.indexes(indexResults.getIndexes());
items.aliases(indexResults.getAliases());
} else {
items.indexes(List.of());
items.aliases(List.of());
log.warn("Stopping before index migration due to issues");
items.failureMessage("Encountered " + metadataResults.fatalIssueCount() + " fatal issue(s) while moving global objects.");
}

return items.build();
}
Expand Down
33 changes: 21 additions & 12 deletions MetadataMigration/src/test/java/org/opensearch/migrations/EndToEndTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import java.io.File;
import java.util.List;
import java.util.concurrent.CompletableFuture;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import org.opensearch.migrations.bulkload.common.FileSystemSnapshotCreator;
Expand All @@ -13,6 +14,7 @@
import org.opensearch.migrations.bulkload.models.DataFilterArgs;
import org.opensearch.migrations.bulkload.worker.SnapshotRunner;
import org.opensearch.migrations.commands.MigrationItemResult;
import org.opensearch.migrations.metadata.CreationResult;
import org.opensearch.migrations.metadata.tracing.MetadataMigrationTestContext;
import org.opensearch.migrations.snapshot.creation.tracing.SnapshotTestContext;

Expand Down Expand Up @@ -43,9 +45,9 @@ class EndToEndTest {

private static Stream<Arguments> scenarios() {
return Stream.of(
Arguments.of(TransferMedium.Http, MetadataCommands.EVALUATE),
Arguments.of(TransferMedium.SnapshotImage, MetadataCommands.MIGRATE),
Arguments.of(TransferMedium.Http, MetadataCommands.MIGRATE)
Arguments.of(TransferMedium.Http, MetadataCommands.EVALUATE)
// Arguments.of(TransferMedium.SnapshotImage, MetadataCommands.MIGRATE),
// Arguments.of(TransferMedium.Http, MetadataCommands.MIGRATE)
);
}

Expand Down Expand Up @@ -132,6 +134,7 @@ private void migrateFrom_ES(
// Creates a document that uses the template
sourceClusterOperations.createDocument(testData.blogIndexName, "222", "{\"author\":\"Tobias Funke\"}");
sourceClusterOperations.createDocument(testData.movieIndexName,"123", "{\"title\":\"This is spinal tap\"}");
sourceClusterOperations.createDocument(testData.indexThatAlreadyExists, "doc66", "{}");

sourceClusterOperations.createAlias(testData.aliasName, "movies*");

Expand Down Expand Up @@ -172,11 +175,13 @@ private void migrateFrom_ES(
arguments.targetArgs.host = targetCluster.getUrl();

var dataFilterArgs = new DataFilterArgs();
dataFilterArgs.indexAllowlist = List.of(testData.blogIndexName, testData.movieIndexName);
dataFilterArgs.indexAllowlist = List.of();
dataFilterArgs.componentTemplateAllowlist = List.of(testData.compoTemplateName);
dataFilterArgs.indexTemplateAllowlist = List.of(testData.indexTemplateName);
arguments.dataFilterArgs = dataFilterArgs;

var targetClusterOperations = new ClusterOperations(targetCluster.getUrl());
targetClusterOperations.createDocument(testData.indexThatAlreadyExists, "doc77", "{}");

// ACTION: Migrate the templates
var metadataContext = MetadataMigrationTestContext.factory().noOtelTracking();
Expand All @@ -191,7 +196,7 @@ private void migrateFrom_ES(

verifyCommandResults(result, sourceIsES6_8, testData);

verifyTargetCluster(targetCluster, command, sourceIsES6_8, testData);
verifyTargetCluster(targetClusterOperations, command, sourceIsES6_8, testData);
}

private static class TestData {
Expand All @@ -201,24 +206,29 @@ private static class TestData {
final String blogIndexName = "blog_2023";
final String movieIndexName = "movies_2023";
final String aliasName = "movies-alias";
final String indexThatAlreadyExists = "already-exists";
}

private void verifyCommandResults(
MigrationItemResult result,
boolean sourceIsES6_8,
TestData testData) {
log.info(result.toString());
log.info(result.asCliOutput());
assertThat(result.getExitCode(), equalTo(0));

var migratedItems = result.getItems();
assertThat(migratedItems.getIndexTemplates(), containsInAnyOrder(testData.indexTemplateName));
assertThat(migratedItems.getComponentTemplates(), equalTo(sourceIsES6_8 ? List.of() : List.of(testData.compoTemplateName)));
assertThat(migratedItems.getIndexes(), containsInAnyOrder(testData.blogIndexName, testData.movieIndexName));
assertThat(migratedItems.getAliases(), containsInAnyOrder(testData.aliasInTemplate, testData.aliasName));
assertThat(getNames(migratedItems.getIndexTemplates()), containsInAnyOrder(testData.indexTemplateName));
assertThat(getNames(migratedItems.getComponentTemplates()), equalTo(sourceIsES6_8 ? List.of() : List.of(testData.compoTemplateName)));
assertThat(getNames(migratedItems.getIndexes()), containsInAnyOrder(testData.blogIndexName, testData.movieIndexName, testData.indexThatAlreadyExists));
assertThat(getNames(migratedItems.getAliases()), containsInAnyOrder(testData.aliasInTemplate, testData.aliasName));
}

private List<String> getNames(List<CreationResult> items) {
return items.stream().map(r -> r.getName()).collect(Collectors.toList());
}

private void verifyTargetCluster(
SearchClusterContainer targetCluster,
ClusterOperations targetClusterOperations,
MetadataCommands command,
boolean sourceIsES6_8,
TestData testData
Expand All @@ -228,7 +238,6 @@ private void verifyTargetCluster(
var verifyResponseCode = expectUpdatesOnTarget ? equalTo(200) : equalTo(404);

// Check that the index was migrated
var targetClusterOperations = new ClusterOperations(targetCluster.getUrl());
var res = targetClusterOperations.get("/" + testData.blogIndexName);
assertThat(res.getValue(), res.getKey(), verifyResponseCode);

Expand Down
56 changes: 51 additions & 5 deletions MetadataMigration/src/test/java/org/opensearch/migrations/cli/ItemsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

import java.util.List;

import org.opensearch.migrations.metadata.CreationResult;
import org.opensearch.migrations.metadata.CreationResult.CreationFailureType;

import org.junit.jupiter.api.Test;

import static org.hamcrest.CoreMatchers.containsString;
Expand Down Expand Up @@ -33,10 +36,22 @@ void testAsString_empty() {
@Test
void testAsString_full() {
var items = Items.builder()
.indexTemplates(List.of("it1", "it2"))
.componentTemplates(List.of("ct1", "ct2"))
.indexes(List.of("i1", "i2"))
.aliases(List.of("a1", "a2"))
.indexTemplates(List.of(
CreationResult.builder().name("it1").build(),
CreationResult.builder().name("it2").build()
))
.componentTemplates(List.of(
CreationResult.builder().name("ct1").build(),
CreationResult.builder().name("ct2").build()
))
.indexes(List.of(
CreationResult.builder().name("i1").build(),
CreationResult.builder().name("i2").build()
))
.aliases(List.of(
CreationResult.builder().name("a1").build(),
CreationResult.builder().name("a2").build()
))
.build();

var result = items.asCliOutput();
Expand All @@ -54,12 +69,43 @@ void testAsString_full() {
assertThat(result, hasLineCount(12));
}

@Test
void testAsString_indexTemplates_failures() {
var items = Items.builder()
.indexTemplates(List.of(
CreationResult.builder().name("it1").failureType(CreationFailureType.ALREADY_EXISTS).build(),
CreationResult.builder().name("it2").failureType(CreationFailureType.TARGET_CLUSTER_FAILURE).exception(new RuntimeException("403 Forbidden")).build()
))
.componentTemplates(List.of())
.indexes(List.of())
.aliases(List.of())
.build();

var result = items.asCliOutput();

assertThat(result, containsString("Migrated Items:"));
assertThat(result, containsString("ERROR - it2 failed on target cluster: 403 Forbidden"));
assertThat(result, containsString("WARN - it1 already exists"));
assertThat(result, containsString("Index Templates:"));
assertThat(result, containsString("Component Templates:"));
assertThat(result, containsString("Indexes:"));
assertThat(result, containsString("Aliases:"));
assertThat(result, containsStringCount(Items.NONE_FOUND_MARKER, 3));
assertThat(result, hasLineCount(13));
}

@Test
void testAsString_itemOrdering() {
var items = Items.builder()
.indexTemplates(List.of())
.componentTemplates(List.of())
.indexes(List.of("i1", "i2", "i5", "i3", "i4"))
.indexes(List.of(
CreationResult.builder().name("i1").build(),
CreationResult.builder().name("i2").build(),
CreationResult.builder().name("i5").build(),
CreationResult.builder().name("i3").build(),
CreationResult.builder().name("i4").build()
))
.aliases(List.of())
.build();

Expand Down
9 changes: 9 additions & 0 deletions ...in/java/org/opensearch/migrations/bulkload/transformers/IndexTransformationException.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package org.opensearch.migrations.bulkload.transformers;

import org.opensearch.migrations.bulkload.common.RfsException;

public class IndexTransformationException extends RfsException {
public IndexTransformationException(String indexName, Throwable cause) {
super("Transformation for index index '" + indexName + "' failed.", cause);
}
}
4 changes: 3 additions & 1 deletion RFS/src/main/java/org/opensearch/migrations/bulkload/transformers/TransformFunctions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,9 @@ else if (val instanceof ObjectNode) {
* it regardless of what it is named.
*/
public static ObjectNode getMappingsFromBeneathIntermediate(ObjectNode mappingsRoot) {
if (mappingsRoot.has(PROPERTIES_KEY_STR)) {
if (mappingsRoot.size() == 0) {
return mappingsRoot;
} else if (mappingsRoot.has(PROPERTIES_KEY_STR)) {
return mappingsRoot;
} else if (!mappingsRoot.has(PROPERTIES_KEY_STR)) {
return (ObjectNode) mappingsRoot.get(mappingsRoot.fieldNames().next()).deepCopy();
Expand Down
Loading

0 comments on commit 7307579

Please sign in to comment.