Skip to content

Commit d94a19a

Browse files
pgomulkapgomulka
committed
Cleanup SystemIndexMigration tests (elastic#84281)
A follow up after elastic#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 elastic#84245
1 parent 3b6af88 commit d94a19a

File tree

7 files changed

+54
-43
lines changed

7 files changed

+54
-43
lines changed

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

Lines changed: 23 additions & 15 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;
@@ -48,6 +49,7 @@
4849
import java.util.function.Function;
4950

5051
import static org.hamcrest.Matchers.containsInAnyOrder;
52+
import static org.hamcrest.Matchers.endsWith;
5153
import static org.hamcrest.Matchers.equalTo;
5254
import static org.hamcrest.Matchers.is;
5355

@@ -73,7 +75,6 @@ public abstract class AbstractFeatureMigrationIntegTest extends ESIntegTestCase
7375
.setAllowedElasticProductOrigins(Collections.singletonList(ORIGIN))
7476
.setMinimumNodeVersion(NEEDS_UPGRADE_VERSION)
7577
.setPriorSystemIndexDescriptors(Collections.emptyList())
76-
7778
.build();
7879
static final SystemIndexDescriptor INTERNAL_UNMANAGED = SystemIndexDescriptor.builder()
7980
.setIndexPattern(".int-unman-*")
@@ -90,8 +91,8 @@ public abstract class AbstractFeatureMigrationIntegTest extends ESIntegTestCase
9091
.setAliasName(".internal-managed-alias")
9192
.setPrimaryIndex(INTERNAL_MANAGED_INDEX_NAME)
9293
.setType(SystemIndexDescriptor.Type.INTERNAL_MANAGED)
93-
.setSettings(createSimpleSettings(NEEDS_UPGRADE_VERSION, INTERNAL_MANAGED_FLAG_VALUE))
94-
.setMappings(createSimpleMapping(true, true, false))
94+
.setSettings(createSettings(NEEDS_UPGRADE_VERSION, INTERNAL_MANAGED_FLAG_VALUE))
95+
.setMappings(createMapping(true, true, false))
9596
.setOrigin(ORIGIN)
9697
.setVersionMetaKey(VERSION_META_KEY)
9798
.setAllowedElasticProductOrigins(Collections.emptyList())
@@ -106,8 +107,8 @@ public abstract class AbstractFeatureMigrationIntegTest extends ESIntegTestCase
106107
.setAliasName(".external-managed-alias")
107108
.setPrimaryIndex(".ext-man-old")
108109
.setType(SystemIndexDescriptor.Type.EXTERNAL_MANAGED)
109-
.setSettings(createSimpleSettings(NEEDS_UPGRADE_VERSION, EXTERNAL_MANAGED_FLAG_VALUE))
110-
.setMappings(createSimpleMapping(true, false, true))
110+
.setSettings(createSettings(NEEDS_UPGRADE_VERSION, EXTERNAL_MANAGED_FLAG_VALUE))
111+
.setMappings(createMapping(true, false, true))
111112
.setOrigin(ORIGIN)
112113
.setVersionMetaKey(VERSION_META_KEY)
113114
.setAllowedElasticProductOrigins(Collections.singletonList(ORIGIN))
@@ -119,14 +120,21 @@ public abstract class AbstractFeatureMigrationIntegTest extends ESIntegTestCase
119120

120121
@Before
121122
public void setupTestPlugin() {
122-
TestPlugin.preMigrationHook.set((state) -> Collections.emptyMap());
123-
TestPlugin.postMigrationHook.set((state, metadata) -> {});
123+
TestPlugin testPlugin = getPlugin(TestPlugin.class);
124+
testPlugin.preMigrationHook.set((state) -> Collections.emptyMap());
125+
testPlugin.postMigrationHook.set((state, metadata) -> {});
126+
}
127+
128+
public <T extends Plugin> T getPlugin(Class<T> type) {
129+
final PluginsService pluginsService = internalCluster().getCurrentMasterNodeInstance(PluginsService.class);
130+
return pluginsService.filterPlugins(type).stream().findFirst().get();
124131
}
125132

126133
public void createSystemIndexForDescriptor(SystemIndexDescriptor descriptor) throws InterruptedException {
127-
Assert.assertTrue(
134+
assertThat(
128135
"the strategy used below to create index names for descriptors without a primary index name only works for simple patterns",
129-
descriptor.getIndexPattern().endsWith("*")
136+
descriptor.getIndexPattern(),
137+
endsWith("*")
130138
);
131139
String indexName = Optional.ofNullable(descriptor.getPrimaryIndex()).orElse(descriptor.getIndexPattern().replace("*", "old"));
132140
CreateIndexRequestBuilder createRequest = prepareCreate(indexName);
@@ -140,14 +148,14 @@ public void createSystemIndexForDescriptor(SystemIndexDescriptor descriptor) thr
140148
);
141149
} else {
142150
createRequest.setSettings(
143-
createSimpleSettings(
151+
createSettings(
144152
NEEDS_UPGRADE_VERSION,
145153
descriptor.isInternal() ? INTERNAL_UNMANAGED_FLAG_VALUE : EXTERNAL_UNMANAGED_FLAG_VALUE
146154
)
147155
);
148156
}
149157
if (descriptor.getMappings() == null) {
150-
createRequest.addMapping("doc", createSimpleMapping(false, descriptor.isInternal(), false), XContentType.JSON);
158+
createRequest.addMapping("doc", createMapping(false, descriptor.isInternal(), false), XContentType.JSON);
151159
}
152160
CreateIndexResponse response = createRequest.get();
153161
Assert.assertTrue(response.isShardsAcknowledged());
@@ -163,7 +171,7 @@ public void createSystemIndexForDescriptor(SystemIndexDescriptor descriptor) thr
163171
Assert.assertThat(indexStats.getIndex(indexName).getTotal().getDocs().getCount(), is((long) INDEX_DOC_COUNT));
164172
}
165173

166-
static Settings createSimpleSettings(Version creationVersion, int flagSettingValue) {
174+
static Settings createSettings(Version creationVersion, int flagSettingValue) {
167175
return Settings.builder()
168176
.put(IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING.getKey(), 1)
169177
.put(IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.getKey(), 0)
@@ -172,7 +180,7 @@ static Settings createSimpleSettings(Version creationVersion, int flagSettingVal
172180
.build();
173181
}
174182

175-
static String createSimpleMapping(boolean descriptorManaged, boolean descriptorInternal, boolean useStandardType) {
183+
static String createMapping(boolean descriptorManaged, boolean descriptorInternal, boolean useStandardType) {
176184
try (XContentBuilder builder = JsonXContent.contentBuilder()) {
177185
builder.startObject();
178186
if (useStandardType) {
@@ -236,8 +244,8 @@ public void assertIndexHasCorrectProperties(
236244
}
237245

238246
public static class TestPlugin extends Plugin implements SystemIndexPlugin {
239-
public static final AtomicReference<Function<ClusterState, Map<String, Object>>> preMigrationHook = new AtomicReference<>();
240-
public static final AtomicReference<BiConsumer<ClusterState, Map<String, Object>>> postMigrationHook = new AtomicReference<>();
247+
public final AtomicReference<Function<ClusterState, Map<String, Object>>> preMigrationHook = new AtomicReference<>();
248+
public final AtomicReference<BiConsumer<ClusterState, Map<String, Object>>> postMigrationHook = new AtomicReference<>();
241249

242250
public TestPlugin() {
243251

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

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

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

8784
PostFeatureUpgradeRequest migrationRequest = new PostFeatureUpgradeRequest();
@@ -132,7 +129,7 @@ public void testMigrateInternalManagedSystemIndex() throws Exception {
132129

133130
SetOnce<Boolean> preUpgradeHookCalled = new SetOnce<>();
134131
SetOnce<Boolean> postUpgradeHookCalled = new SetOnce<>();
135-
TestPlugin.preMigrationHook.set(clusterState -> {
132+
getPlugin(TestPlugin.class).preMigrationHook.set(clusterState -> {
136133
// Check that the ordering of these calls is correct.
137134
assertThat(postUpgradeHookCalled.get(), nullValue());
138135
Map<String, Object> metadata = new HashMap<>();
@@ -149,7 +146,7 @@ public void testMigrateInternalManagedSystemIndex() throws Exception {
149146
return metadata;
150147
});
151148

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

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

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

250244
client().execute(PostFeatureUpgradeAction.INSTANCE, new PostFeatureUpgradeRequest()).get();
@@ -262,9 +256,6 @@ public void testMigrateIndexWithWriteBlock() throws Exception {
262256
public void testMigrationWillRunAfterError() throws Exception {
263257
createSystemIndexForDescriptor(INTERNAL_MANAGED);
264258

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

270261
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_6_0_0, 0))
267-
.setMappings(createSimpleMapping(true, true, true))
266+
.setSettings(createSettings(Version.V_6_0_0, 0))
267+
.setMappings(createMapping(true, 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)