Skip to content

Commit 3d3eb31

Browse files
pgomulkapgomulka
authored
Cleanup SystemIndexMigration tests (#84281)
A follow up after #84192 refactor the static state in TestPlugin to be an instance refactor assertions to use hamcrest remove Simple from methods as it is not meaningful refactor xcontent tests to support unknown fields closes #84245
1 parent 3b6ea69 commit 3d3eb31

File tree

7 files changed

+54
-42
lines changed

7 files changed

+54
-42
lines changed

‎modules/reindex/src/internalClusterTest/java/org/elasticsearch/migration/AbstractFeatureMigrationIntegTest.java‎

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.elasticsearch.indices.AssociatedIndexDescriptor;
2727
import org.elasticsearch.indices.SystemIndexDescriptor;
2828
import org.elasticsearch.plugins.Plugin;
29+
import org.elasticsearch.plugins.PluginsService;
2930
import org.elasticsearch.plugins.SystemIndexPlugin;
3031
import org.elasticsearch.test.ESIntegTestCase;
3132
import org.elasticsearch.xcontent.XContentBuilder;
@@ -47,6 +48,7 @@
4748
import java.util.function.Function;
4849

4950
import static org.hamcrest.Matchers.containsInAnyOrder;
51+
import static org.hamcrest.Matchers.endsWith;
5052
import static org.hamcrest.Matchers.equalTo;
5153
import static org.hamcrest.Matchers.is;
5254

@@ -88,8 +90,8 @@ public abstract class AbstractFeatureMigrationIntegTest extends ESIntegTestCase
8890
.setAliasName(".internal-managed-alias")
8991
.setPrimaryIndex(INTERNAL_MANAGED_INDEX_NAME)
9092
.setType(SystemIndexDescriptor.Type.INTERNAL_MANAGED)
91-
.setSettings(createSimpleSettings(NEEDS_UPGRADE_VERSION, INTERNAL_MANAGED_FLAG_VALUE))
92-
.setMappings(createSimpleMapping(true, true))
93+
.setSettings(createSettings(NEEDS_UPGRADE_VERSION, INTERNAL_MANAGED_FLAG_VALUE))
94+
.setMappings(createMapping(true, true))
9395
.setOrigin(ORIGIN)
9496
.setVersionMetaKey(VERSION_META_KEY)
9597
.setAllowedElasticProductOrigins(Collections.emptyList())
@@ -103,8 +105,8 @@ public abstract class AbstractFeatureMigrationIntegTest extends ESIntegTestCase
103105
.setAliasName(".external-managed-alias")
104106
.setPrimaryIndex(".ext-man-old")
105107
.setType(SystemIndexDescriptor.Type.EXTERNAL_MANAGED)
106-
.setSettings(createSimpleSettings(NEEDS_UPGRADE_VERSION, EXTERNAL_MANAGED_FLAG_VALUE))
107-
.setMappings(createSimpleMapping(true, false))
108+
.setSettings(createSettings(NEEDS_UPGRADE_VERSION, EXTERNAL_MANAGED_FLAG_VALUE))
109+
.setMappings(createMapping(true, false))
108110
.setOrigin(ORIGIN)
109111
.setVersionMetaKey(VERSION_META_KEY)
110112
.setAllowedElasticProductOrigins(Collections.singletonList(ORIGIN))
@@ -116,22 +118,29 @@ public abstract class AbstractFeatureMigrationIntegTest extends ESIntegTestCase
116118

117119
@Before
118120
public void setupTestPlugin() {
119-
TestPlugin.preMigrationHook.set((state) -> Collections.emptyMap());
120-
TestPlugin.postMigrationHook.set((state, metadata) -> {});
121+
TestPlugin testPlugin = getPlugin(TestPlugin.class);
122+
testPlugin.preMigrationHook.set((state) -> Collections.emptyMap());
123+
testPlugin.postMigrationHook.set((state, metadata) -> {});
124+
}
125+
126+
public <T extends Plugin> T getPlugin(Class<T> type) {
127+
final PluginsService pluginsService = internalCluster().getCurrentMasterNodeInstance(PluginsService.class);
128+
return pluginsService.filterPlugins(type).stream().findFirst().get();
121129
}
122130

123131
public void createSystemIndexForDescriptor(SystemIndexDescriptor descriptor) throws InterruptedException {
124-
Assert.assertTrue(
132+
assertThat(
125133
"the strategy used below to create index names for descriptors without a primary index name only works for simple patterns",
126-
descriptor.getIndexPattern().endsWith("*")
134+
descriptor.getIndexPattern(),
135+
endsWith("*")
127136
);
128137
String indexName = Optional.ofNullable(descriptor.getPrimaryIndex()).orElse(descriptor.getIndexPattern().replace("*", "old"));
129138
CreateIndexRequestBuilder createRequest = prepareCreate(indexName);
130139
createRequest.setWaitForActiveShards(ActiveShardCount.ALL);
131140
if (SystemIndexDescriptor.DEFAULT_SETTINGS.equals(descriptor.getSettings())) {
132141
// unmanaged
133142
createRequest.setSettings(
134-
createSimpleSettings(
143+
createSettings(
135144
NEEDS_UPGRADE_VERSION,
136145
descriptor.isInternal() ? INTERNAL_UNMANAGED_FLAG_VALUE : EXTERNAL_UNMANAGED_FLAG_VALUE
137146
)
@@ -146,7 +155,7 @@ public void createSystemIndexForDescriptor(SystemIndexDescriptor descriptor) thr
146155
);
147156
}
148157
if (descriptor.getMappings() == null) {
149-
createRequest.setMapping(createSimpleMapping(false, descriptor.isInternal()));
158+
createRequest.setMapping(createMapping(false, descriptor.isInternal()));
150159
}
151160
CreateIndexResponse response = createRequest.get();
152161
Assert.assertTrue(response.isShardsAcknowledged());
@@ -160,7 +169,7 @@ public void createSystemIndexForDescriptor(SystemIndexDescriptor descriptor) thr
160169
Assert.assertThat(indexStats.getIndex(indexName).getTotal().getDocs().getCount(), is((long) INDEX_DOC_COUNT));
161170
}
162171

163-
static Settings createSimpleSettings(Version creationVersion, int flagSettingValue) {
172+
static Settings createSettings(Version creationVersion, int flagSettingValue) {
164173
return Settings.builder()
165174
.put(IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING.getKey(), 1)
166175
.put(IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.getKey(), 0)
@@ -169,7 +178,7 @@ static Settings createSimpleSettings(Version creationVersion, int flagSettingVal
169178
.build();
170179
}
171180

172-
static String createSimpleMapping(boolean descriptorManaged, boolean descriptorInternal) {
181+
static String createMapping(boolean descriptorManaged, boolean descriptorInternal) {
173182
try (XContentBuilder builder = JsonXContent.contentBuilder()) {
174183
builder.startObject();
175184
{
@@ -227,8 +236,8 @@ public void assertIndexHasCorrectProperties(
227236
}
228237

229238
public static class TestPlugin extends Plugin implements SystemIndexPlugin {
230-
public static final AtomicReference<Function<ClusterState, Map<String, Object>>> preMigrationHook = new AtomicReference<>();
231-
public static final AtomicReference<BiConsumer<ClusterState, Map<String, Object>>> postMigrationHook = new AtomicReference<>();
239+
public final AtomicReference<Function<ClusterState, Map<String, Object>>> preMigrationHook = new AtomicReference<>();
240+
public final AtomicReference<BiConsumer<ClusterState, Map<String, Object>>> postMigrationHook = new AtomicReference<>();
232241

233242
public TestPlugin() {
234243

‎modules/reindex/src/internalClusterTest/java/org/elasticsearch/migration/FeatureMigrationIT.java‎

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,6 @@ public void testStartMigrationAndImmediatelyCheckStatus() throws Exception {
8080
createSystemIndexForDescriptor(EXTERNAL_MANAGED);
8181
createSystemIndexForDescriptor(EXTERNAL_UNMANAGED);
8282

83-
TestPlugin.preMigrationHook.set((state) -> Collections.emptyMap());
84-
TestPlugin.postMigrationHook.set((state, metadata) -> {});
85-
8683
ensureGreen();
8784

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

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

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

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

246-
TestPlugin.preMigrationHook.set((state) -> Collections.emptyMap());
247-
TestPlugin.postMigrationHook.set((state, metadata) -> {});
248-
249243
ensureGreen();
250244

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

266-
TestPlugin.preMigrationHook.set((state) -> Collections.emptyMap());
267-
TestPlugin.postMigrationHook.set((state, metadata) -> {});
268-
269260
ensureGreen();
270261

271262
SetOnce<Exception> failure = new SetOnce<>();

‎modules/reindex/src/internalClusterTest/java/org/elasticsearch/migration/MultiFeatureMigrationIT.java‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ public void testMultipleFeatureMigration() throws Exception {
100100
SetOnce<Boolean> secondPluginPreMigrationHookCalled = new SetOnce<>();
101101
SetOnce<Boolean> secondPluginPostMigrationHookCalled = new SetOnce<>();
102102

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

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

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

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

275275
public static class SecondPlugin extends Plugin implements SystemIndexPlugin {
276276

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

280-
public SecondPlugin() {
281-
282-
}
280+
public SecondPlugin() {}
283281

284282
@Override
285283
public String getFeatureName() {

‎modules/reindex/src/internalClusterTest/java/org/elasticsearch/migration/SystemIndexMigrationIT.java‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.elasticsearch.reindex.ReindexPlugin;
2525
import org.elasticsearch.test.ESIntegTestCase;
2626
import org.elasticsearch.test.InternalTestCluster;
27+
import org.junit.Before;
2728

2829
import java.util.ArrayList;
2930
import java.util.Collection;
@@ -63,15 +64,20 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
6364
return plugins;
6465
}
6566

66-
public void testSystemIndexMigrationCanBeInterruptedWithShutdown() throws Exception {
67+
@Before
68+
public void init() {
69+
// master node is created in AbstractFeatureMigrationIntegTest#setupTestPlugin when trying to access plugins
70+
internalCluster().setBootstrapMasterNodeIndex(0);
71+
}
6772

73+
public void testSystemIndexMigrationCanBeInterruptedWithShutdown() throws Exception {
6874
CyclicBarrier taskCreated = new CyclicBarrier(2);
6975
CyclicBarrier shutdownCompleted = new CyclicBarrier(2);
7076
AtomicBoolean hasBlocked = new AtomicBoolean();
7177

72-
internalCluster().setBootstrapMasterNodeIndex(0);
73-
final String masterName = internalCluster().startMasterOnlyNode();
78+
final String masterName = internalCluster().getMasterName();
7479
final String masterAndDataNode = internalCluster().startNode();
80+
7581
createSystemIndexForDescriptor(INTERNAL_MANAGED);
7682

7783
final ClusterStateListener clusterStateListener = event -> {

‎server/src/main/java/org/elasticsearch/upgrades/SystemIndexMigrationTaskState.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@
3333
public class SystemIndexMigrationTaskState implements PersistentTaskState {
3434
private static final ParseField CURRENT_INDEX_FIELD = new ParseField("current_index");
3535
private static final ParseField CURRENT_FEATURE_FIELD = new ParseField("current_feature");
36-
private static final ParseField FEATURE_METADATA_MAP_FIELD = new ParseField("feature_metadata");
36+
// scope for testing
37+
static final ParseField FEATURE_METADATA_MAP_FIELD = new ParseField("feature_metadata");
3738

3839
@SuppressWarnings(value = "unchecked")
3940
static final ConstructingObjectParser<SystemIndexMigrationTaskState, Void> PARSER = new ConstructingObjectParser<>(

‎server/src/test/java/org/elasticsearch/upgrades/SystemIndexMigrationTaskParamsXContentTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ protected SystemIndexMigrationTaskParams doParseInstance(XContentParser parser)
2828

2929
@Override
3030
protected boolean supportsUnknownFields() {
31-
return false;
31+
return true;
3232
}
3333

3434
@Override

‎server/src/test/java/org/elasticsearch/upgrades/SystemIndexMigrationTaskStateXContentTests.java‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import org.elasticsearch.xcontent.XContentParser;
1414

1515
import java.io.IOException;
16+
import java.util.function.Predicate;
1617

1718
public class SystemIndexMigrationTaskStateXContentTests extends AbstractXContentTestCase<SystemIndexMigrationTaskState> {
1819

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

2930
@Override
3031
protected boolean supportsUnknownFields() {
31-
return false;
32+
return true;
33+
}
34+
35+
@Override
36+
protected Predicate<String> getRandomFieldsExcludeFilter() {
37+
// featureCallbackMetadata is a Map<String,Object> so adding random fields there make no sense
38+
return p -> p.startsWith(SystemIndexMigrationTaskState.FEATURE_METADATA_MAP_FIELD.getPreferredName());
3239
}
3340

3441
@Override

0 commit comments

Comments
 (0)