Skip to content

Commit

Permalink
[8.1] Cleanup SystemIndexMigration tests (#84281) (#85153)
Browse files Browse the repository at this point in the history 
backports
* Cleanup SystemIndexMigration tests (#84281)
* Fix randomness in SystemIndexMigrationIT (#85224)
  • Loading branch information
@pgomulka
pgomulka committed Mar 23, 2022
1 parent 772cd96 commit fff4c22
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 46 deletions.
47 changes: 32 additions & 15 deletions ...ternalClusterTest/java/org/elasticsearch/migration/AbstractFeatureMigrationIntegTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.elasticsearch.indices.AssociatedIndexDescriptor;
import org.elasticsearch.indices.SystemIndexDescriptor;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.plugins.PluginsService;
import org.elasticsearch.plugins.SystemIndexPlugin;
import org.elasticsearch.test.ESIntegTestCase;
import org.elasticsearch.xcontent.XContentBuilder;
Expand All @@ -47,9 +48,11 @@
import java.util.function.Function;

import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.endsWith;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;

@ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST, numDataNodes = 0, numClientNodes = 0, autoManageMasterNodes = false)
public abstract class AbstractFeatureMigrationIntegTest extends ESIntegTestCase {

static final String VERSION_META_KEY = "version";
Expand Down Expand Up @@ -88,8 +91,8 @@ public abstract class AbstractFeatureMigrationIntegTest extends ESIntegTestCase
.setAliasName(".internal-managed-alias")
.setPrimaryIndex(INTERNAL_MANAGED_INDEX_NAME)
.setType(SystemIndexDescriptor.Type.INTERNAL_MANAGED)
.setSettings(createSimpleSettings(NEEDS_UPGRADE_VERSION, INTERNAL_MANAGED_FLAG_VALUE))
.setMappings(createSimpleMapping(true, true))
.setSettings(createSettings(NEEDS_UPGRADE_VERSION, INTERNAL_MANAGED_FLAG_VALUE))
.setMappings(createMapping(true, true))
.setOrigin(ORIGIN)
.setVersionMetaKey(VERSION_META_KEY)
.setAllowedElasticProductOrigins(Collections.emptyList())
Expand All @@ -103,8 +106,8 @@ public abstract class AbstractFeatureMigrationIntegTest extends ESIntegTestCase
.setAliasName(".external-managed-alias")
.setPrimaryIndex(".ext-man-old")
.setType(SystemIndexDescriptor.Type.EXTERNAL_MANAGED)
.setSettings(createSimpleSettings(NEEDS_UPGRADE_VERSION, EXTERNAL_MANAGED_FLAG_VALUE))
.setMappings(createSimpleMapping(true, false))
.setSettings(createSettings(NEEDS_UPGRADE_VERSION, EXTERNAL_MANAGED_FLAG_VALUE))
.setMappings(createMapping(true, false))
.setOrigin(ORIGIN)
.setVersionMetaKey(VERSION_META_KEY)
.setAllowedElasticProductOrigins(Collections.singletonList(ORIGIN))
Expand All @@ -114,24 +117,38 @@ public abstract class AbstractFeatureMigrationIntegTest extends ESIntegTestCase
static final int EXTERNAL_UNMANAGED_FLAG_VALUE = 4;
static final String ASSOCIATED_INDEX_NAME = ".my-associated-idx";

protected String masterAndDataNode;
protected String masterName;

@Before
public void setupTestPlugin() {
TestPlugin.preMigrationHook.set((state) -> Collections.emptyMap());
TestPlugin.postMigrationHook.set((state, metadata) -> {});
public void setup() {
internalCluster().setBootstrapMasterNodeIndex(0);
masterName = internalCluster().startMasterOnlyNode();
masterAndDataNode = internalCluster().startNode();

TestPlugin testPlugin = getPlugin(TestPlugin.class);
testPlugin.preMigrationHook.set((state) -> Collections.emptyMap());
testPlugin.postMigrationHook.set((state, metadata) -> {});
}

public <T extends Plugin> T getPlugin(Class<T> type) {
final PluginsService pluginsService = internalCluster().getCurrentMasterNodeInstance(PluginsService.class);
return pluginsService.filterPlugins(type).stream().findFirst().get();
}

public void createSystemIndexForDescriptor(SystemIndexDescriptor descriptor) throws InterruptedException {
Assert.assertTrue(
assertThat(
"the strategy used below to create index names for descriptors without a primary index name only works for simple patterns",
descriptor.getIndexPattern().endsWith("*")
descriptor.getIndexPattern(),
endsWith("*")
);
String indexName = Optional.ofNullable(descriptor.getPrimaryIndex()).orElse(descriptor.getIndexPattern().replace("*", "old"));
CreateIndexRequestBuilder createRequest = prepareCreate(indexName);
createRequest.setWaitForActiveShards(ActiveShardCount.ALL);
if (SystemIndexDescriptor.DEFAULT_SETTINGS.equals(descriptor.getSettings())) {
// unmanaged
createRequest.setSettings(
createSimpleSettings(
createSettings(
NEEDS_UPGRADE_VERSION,
descriptor.isInternal() ? INTERNAL_UNMANAGED_FLAG_VALUE : EXTERNAL_UNMANAGED_FLAG_VALUE
)
Expand All @@ -146,7 +163,7 @@ public void createSystemIndexForDescriptor(SystemIndexDescriptor descriptor) thr
);
}
if (descriptor.getMappings() == null) {
createRequest.setMapping(createSimpleMapping(false, descriptor.isInternal()));
createRequest.setMapping(createMapping(false, descriptor.isInternal()));
}
CreateIndexResponse response = createRequest.get();
Assert.assertTrue(response.isShardsAcknowledged());
Expand All @@ -160,7 +177,7 @@ public void createSystemIndexForDescriptor(SystemIndexDescriptor descriptor) thr
Assert.assertThat(indexStats.getIndex(indexName).getTotal().getDocs().getCount(), is((long) INDEX_DOC_COUNT));
}

static Settings createSimpleSettings(Version creationVersion, int flagSettingValue) {
static Settings createSettings(Version creationVersion, int flagSettingValue) {
return Settings.builder()
.put(IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING.getKey(), 1)
.put(IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.getKey(), 0)
Expand All @@ -169,7 +186,7 @@ static Settings createSimpleSettings(Version creationVersion, int flagSettingVal
.build();
}

static String createSimpleMapping(boolean descriptorManaged, boolean descriptorInternal) {
static String createMapping(boolean descriptorManaged, boolean descriptorInternal) {
try (XContentBuilder builder = JsonXContent.contentBuilder()) {
builder.startObject();
{
Expand Down Expand Up @@ -227,8 +244,8 @@ public void assertIndexHasCorrectProperties(
}

public static class TestPlugin extends Plugin implements SystemIndexPlugin {
public static final AtomicReference<Function<ClusterState, Map<String, Object>>> preMigrationHook = new AtomicReference<>();
public static final AtomicReference<BiConsumer<ClusterState, Map<String, Object>>> postMigrationHook = new AtomicReference<>();
public final AtomicReference<Function<ClusterState, Map<String, Object>>> preMigrationHook = new AtomicReference<>();
public final AtomicReference<BiConsumer<ClusterState, Map<String, Object>>> postMigrationHook = new AtomicReference<>();

public TestPlugin() {

Expand Down
13 changes: 2 additions & 11 deletions .../reindex/src/internalClusterTest/java/org/elasticsearch/migration/FeatureMigrationIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,6 @@ public void testStartMigrationAndImmediatelyCheckStatus() throws Exception {
createSystemIndexForDescriptor(EXTERNAL_MANAGED);
createSystemIndexForDescriptor(EXTERNAL_UNMANAGED);

TestPlugin.preMigrationHook.set((state) -> Collections.emptyMap());
TestPlugin.postMigrationHook.set((state, metadata) -> {});

ensureGreen();

PostFeatureUpgradeRequest migrationRequest = new PostFeatureUpgradeRequest();
Expand Down Expand Up @@ -133,7 +130,7 @@ public void testMigrateInternalManagedSystemIndex() throws Exception {

SetOnce<Boolean> preUpgradeHookCalled = new SetOnce<>();
SetOnce<Boolean> postUpgradeHookCalled = new SetOnce<>();
TestPlugin.preMigrationHook.set(clusterState -> {
getPlugin(TestPlugin.class).preMigrationHook.set(clusterState -> {
// Check that the ordering of these calls is correct.
assertThat(postUpgradeHookCalled.get(), nullValue());
Map<String, Object> metadata = new HashMap<>();
Expand All @@ -150,7 +147,7 @@ public void testMigrateInternalManagedSystemIndex() throws Exception {
return metadata;
});

TestPlugin.postMigrationHook.set((clusterState, metadata) -> {
getPlugin(TestPlugin.class).postMigrationHook.set((clusterState, metadata) -> {
assertThat(preUpgradeHookCalled.get(), is(true));

assertThat(metadata, hasEntry("stringKey", "stringValue"));
Expand Down Expand Up @@ -243,9 +240,6 @@ public void testMigrateIndexWithWriteBlock() throws Exception {
.orElse(INTERNAL_UNMANAGED.getIndexPattern().replace("*", "old"));
client().admin().indices().prepareUpdateSettings(indexName).setSettings(Settings.builder().put("index.blocks.write", true)).get();

TestPlugin.preMigrationHook.set((state) -> Collections.emptyMap());
TestPlugin.postMigrationHook.set((state, metadata) -> {});

ensureGreen();

client().execute(PostFeatureUpgradeAction.INSTANCE, new PostFeatureUpgradeRequest()).get();
Expand All @@ -263,9 +257,6 @@ public void testMigrateIndexWithWriteBlock() throws Exception {
public void testMigrationWillRunAfterError() throws Exception {
createSystemIndexForDescriptor(INTERNAL_MANAGED);

TestPlugin.preMigrationHook.set((state) -> Collections.emptyMap());
TestPlugin.postMigrationHook.set((state, metadata) -> {});

ensureGreen();

SetOnce<Exception> failure = new SetOnce<>();
Expand Down
20 changes: 9 additions & 11 deletions ...dex/src/internalClusterTest/java/org/elasticsearch/migration/MultiFeatureMigrationIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public void testMultipleFeatureMigration() throws Exception {
SetOnce<Boolean> secondPluginPreMigrationHookCalled = new SetOnce<>();
SetOnce<Boolean> secondPluginPostMigrationHookCalled = new SetOnce<>();

TestPlugin.preMigrationHook.set(clusterState -> {
getPlugin(TestPlugin.class).preMigrationHook.set(clusterState -> {
// None of the other hooks should have been called yet.
assertThat(postMigrationHookCalled.get(), nullValue());
assertThat(secondPluginPreMigrationHookCalled.get(), nullValue());
Expand All @@ -117,7 +117,7 @@ public void testMultipleFeatureMigration() throws Exception {
return metadata;
});

TestPlugin.postMigrationHook.set((clusterState, metadata) -> {
getPlugin(TestPlugin.class).postMigrationHook.set((clusterState, metadata) -> {
// Check that the hooks have been called or not as expected.
assertThat(preMigrationHookCalled.get(), is(true));
assertThat(secondPluginPreMigrationHookCalled.get(), nullValue());
Expand All @@ -133,7 +133,7 @@ public void testMultipleFeatureMigration() throws Exception {
hooksCalled.countDown();
});

SecondPlugin.preMigrationHook.set(clusterState -> {
getPlugin(SecondPlugin.class).preMigrationHook.set(clusterState -> {
// Check that the hooks have been called or not as expected.
assertThat(preMigrationHookCalled.get(), is(true));
assertThat(postMigrationHookCalled.get(), is(true));
Expand All @@ -155,7 +155,7 @@ public void testMultipleFeatureMigration() throws Exception {
return metadata;
});

SecondPlugin.postMigrationHook.set((clusterState, metadata) -> {
getPlugin(SecondPlugin.class).postMigrationHook.set((clusterState, metadata) -> {
// Check that the hooks have been called or not as expected.
assertThat(preMigrationHookCalled.get(), is(true));
assertThat(postMigrationHookCalled.get(), is(true));
Expand Down Expand Up @@ -263,8 +263,8 @@ public void testMultipleFeatureMigration() throws Exception {
.setAliasName(".second-internal-managed-alias")
.setPrimaryIndex(".second-int-man-old")
.setType(SystemIndexDescriptor.Type.INTERNAL_MANAGED)
.setSettings(createSimpleSettings(Version.V_7_0_0, 0))
.setMappings(createSimpleMapping(true, true))
.setSettings(createSettings(Version.V_7_0_0, 0))
.setMappings(createMapping(true, true))
.setOrigin(ORIGIN)
.setVersionMetaKey(VERSION_META_KEY)
.setAllowedElasticProductOrigins(Collections.emptyList())
Expand All @@ -274,12 +274,10 @@ public void testMultipleFeatureMigration() throws Exception {

public static class SecondPlugin extends Plugin implements SystemIndexPlugin {

private static final AtomicReference<Function<ClusterState, Map<String, Object>>> preMigrationHook = new AtomicReference<>();
private static final AtomicReference<BiConsumer<ClusterState, Map<String, Object>>> postMigrationHook = new AtomicReference<>();
private final AtomicReference<Function<ClusterState, Map<String, Object>>> preMigrationHook = new AtomicReference<>();
private final AtomicReference<BiConsumer<ClusterState, Map<String, Object>>> postMigrationHook = new AtomicReference<>();

public SecondPlugin() {

}
public SecondPlugin() {}

@Override
public String getFeatureName() {
Expand Down
6 changes: 0 additions & 6 deletions ...ndex/src/internalClusterTest/java/org/elasticsearch/migration/SystemIndexMigrationIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import org.elasticsearch.persistent.PersistentTasksCustomMetadata;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.reindex.ReindexPlugin;
import org.elasticsearch.test.ESIntegTestCase;
import org.elasticsearch.test.InternalTestCluster;

import java.util.ArrayList;
Expand All @@ -40,7 +39,6 @@
/**
* This class is for testing that when restarting a node, SystemIndexMigrationTaskState can be read.
*/
@ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST, numDataNodes = 0, numClientNodes = 0, autoManageMasterNodes = false)
public class SystemIndexMigrationIT extends AbstractFeatureMigrationIntegTest {
private static Logger logger = LogManager.getLogger(SystemIndexMigrationIT.class);

Expand All @@ -64,14 +62,10 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
}

public void testSystemIndexMigrationCanBeInterruptedWithShutdown() throws Exception {

CyclicBarrier taskCreated = new CyclicBarrier(2);
CyclicBarrier shutdownCompleted = new CyclicBarrier(2);
AtomicBoolean hasBlocked = new AtomicBoolean();

internalCluster().setBootstrapMasterNodeIndex(0);
final String masterName = internalCluster().startMasterOnlyNode();
final String masterAndDataNode = internalCluster().startNode();
createSystemIndexForDescriptor(INTERNAL_MANAGED);

final ClusterStateListener clusterStateListener = event -> {
Expand Down
3 changes: 2 additions & 1 deletion server/src/main/java/org/elasticsearch/upgrades/SystemIndexMigrationTaskState.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@
public class SystemIndexMigrationTaskState implements PersistentTaskState {
private static final ParseField CURRENT_INDEX_FIELD = new ParseField("current_index");
private static final ParseField CURRENT_FEATURE_FIELD = new ParseField("current_feature");
private static final ParseField FEATURE_METADATA_MAP_FIELD = new ParseField("feature_metadata");
// scope for testing
static final ParseField FEATURE_METADATA_MAP_FIELD = new ParseField("feature_metadata");

@SuppressWarnings(value = "unchecked")
static final ConstructingObjectParser<SystemIndexMigrationTaskState, Void> PARSER = new ConstructingObjectParser<>(
Expand Down
2 changes: 1 addition & 1 deletion ...src/test/java/org/elasticsearch/upgrades/SystemIndexMigrationTaskParamsXContentTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ protected SystemIndexMigrationTaskParams doParseInstance(XContentParser parser)

@Override
protected boolean supportsUnknownFields() {
return false;
return true;
}

@Override
Expand Down
9 changes: 8 additions & 1 deletion .../src/test/java/org/elasticsearch/upgrades/SystemIndexMigrationTaskStateXContentTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import org.elasticsearch.xcontent.XContentParser;

import java.io.IOException;
import java.util.function.Predicate;

public class SystemIndexMigrationTaskStateXContentTests extends AbstractXContentTestCase<SystemIndexMigrationTaskState> {

Expand All @@ -28,7 +29,13 @@ protected SystemIndexMigrationTaskState doParseInstance(XContentParser parser) t

@Override
protected boolean supportsUnknownFields() {
return false;
return true;
}

@Override
protected Predicate<String> getRandomFieldsExcludeFilter() {
// featureCallbackMetadata is a Map<String,Object> so adding random fields there make no sense
return p -> p.startsWith(SystemIndexMigrationTaskState.FEATURE_METADATA_MAP_FIELD.getPreferredName());
}

@Override
Expand Down

0 comments on commit fff4c22

Please sign in to comment.