Reenable SearchableSnapshotsRollingUpgradeIT (#69788)

tlrx · web-flow · commit 2ed8e16c4c78 · 2021-03-02T12:33:22.000+01:00
The test SearchableSnapshotsRollingUpgradeIT caused CI failures at least two times (#69705) since it was merged into master. After looking at all node logs, I wasn't able to determine precisely the cause of the test suite timeout. After looking at the code I think it is preferable to use a dedicated path.repo for this test so that it does not interfere with existing and future tests that could wipe the repositories and snapshots of the default path.repo at any time. I also fixed an issue with the storage paramater that should only work starting 7.12.0. Finally, #69704 has been merged and it should provide more nformation if this test fails again. Closes #69705
diff --git a/x-pack/qa/rolling-upgrade/build.gradle b/x-pack/qa/rolling-upgrade/build.gradle
@@ -33,7 +33,9 @@ tasks.register("copyTestNodeKeyMaterial", Copy) {
 
 for (Version bwcVersion : BuildParams.bwcVersions.wireCompatible) {
   String baseName = "v${bwcVersion}"
-  String repositoryPath = "${buildDir}/cluster/shared/repo/${baseName}"
+
+  // SearchableSnapshotsRollingUpgradeIT uses a specific repository to not interfere with other tests
+  String searchableSnapshotRepository = "${buildDir}/cluster/shared/searchable-snapshots-repo/${baseName}"
 
   testClusters {
     "${baseName}" {
@@ -42,7 +44,7 @@ for (Version bwcVersion : BuildParams.bwcVersions.wireCompatible) {
       numberOfNodes = 3
 
       setting 'repositories.url.allowed_urls', 'http://snapshot.test*'
-      setting 'path.repo', repositoryPath
+      setting 'path.repo', "[ \"${buildDir}/cluster/shared/repo/${baseName}\", \"${searchableSnapshotRepository}\" ]"
       setting 'xpack.license.self_generated.type', 'trial'
       setting 'xpack.security.enabled', 'true'
       setting 'xpack.security.transport.ssl.enabled', 'true'
@@ -94,7 +96,7 @@ for (Version bwcVersion : BuildParams.bwcVersions.wireCompatible) {
     dependsOn "copyTestNodeKeyMaterial"
     systemProperty 'tests.rest.suite', 'old_cluster'
     systemProperty 'tests.upgrade_from_version', version.toString().replace('-SNAPSHOT', '')
-    systemProperty 'tests.path.repo', repositoryPath
+    systemProperty 'tests.path.searchable.snapshots.repo', searchableSnapshotRepository
     nonInputProperties.systemProperty('tests.rest.cluster', "${-> testClusters."${baseName}".allHttpSocketURI.join(",")}")
     nonInputProperties.systemProperty('tests.clustername', "${-> testClusters."${baseName}".getName()}")
   }
@@ -111,7 +113,7 @@ for (Version bwcVersion : BuildParams.bwcVersions.wireCompatible) {
     systemProperty 'tests.rest.suite', 'mixed_cluster'
     systemProperty 'tests.first_round', 'true'
     systemProperty 'tests.upgrade_from_version', oldVersion
-    systemProperty 'tests.path.repo', repositoryPath
+    systemProperty 'tests.path.searchable.snapshots.repo', searchableSnapshotRepository
     // We only need to run these tests once so we may as well do it when we're two thirds upgraded
     systemProperty 'tests.rest.blacklist', [
       'mixed_cluster/10_basic/Start scroll in mixed cluster on upgraded node that we will continue after upgrade',
@@ -137,7 +139,7 @@ for (Version bwcVersion : BuildParams.bwcVersions.wireCompatible) {
     systemProperty 'tests.rest.suite', 'mixed_cluster'
     systemProperty 'tests.first_round', 'false'
     systemProperty 'tests.upgrade_from_version', oldVersion
-    systemProperty 'tests.path.repo', repositoryPath
+    systemProperty 'tests.path.searchable.snapshots.repo', searchableSnapshotRepository
   }
 
   tasks.register("${baseName}#upgradedClusterTest", StandaloneRestIntegTestTask) {
@@ -150,7 +152,7 @@ for (Version bwcVersion : BuildParams.bwcVersions.wireCompatible) {
     nonInputProperties.systemProperty('tests.clustername', "${-> testClusters."${baseName}".getName()}")
     systemProperty 'tests.rest.suite', 'upgraded_cluster'
     systemProperty 'tests.upgrade_from_version', oldVersion
-    systemProperty 'tests.path.repo', repositoryPath
+    systemProperty 'tests.path.searchable.snapshots.repo', searchableSnapshotRepository
   }
 
   tasks.register(bwcTaskName(bwcVersion)) {
diff --git a/x-pack/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/SearchableSnapshotsRollingUpgradeIT.java b/x-pack/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/SearchableSnapshotsRollingUpgradeIT.java
@@ -11,7 +11,6 @@
 import org.apache.http.client.methods.HttpGet;
 import org.apache.http.client.methods.HttpPost;
 import org.apache.http.client.methods.HttpPut;
-import org.apache.lucene.util.LuceneTestCase;
 import org.elasticsearch.Version;
 import org.elasticsearch.client.Request;
 import org.elasticsearch.client.Response;
@@ -34,7 +33,6 @@
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.notNullValue;
 
-@LuceneTestCase.AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/69705")
 public class SearchableSnapshotsRollingUpgradeIT extends AbstractUpgradeTestCase {
 
     public void testMountFullCopyAndRecoversCorrectly() throws Exception {
@@ -56,18 +54,16 @@ public void testMountPartialCopyAndRecoversCorrectly() throws Exception {
      */
     private void executeMountAndRecoversCorrectlyTestCase(Storage storage, long numberOfDocs) throws Exception {
         final String suffix = storage.storageName().toLowerCase(Locale.ROOT);
+        final String repository = "repository_" + suffix;
+        final String snapshot = "snapshot_" + suffix;
         final String index = "mounted_index_" + suffix;
 
         if (CLUSTER_TYPE.equals(ClusterType.OLD)) {
-            final String repository = "repository_" + suffix;
-            final String snapshot = "snapshot_" + suffix;
-
-            registerRepository(repository, FsRepository.TYPE, true,
-                Settings.builder().put("location", System.getProperty("tests.path.repo") + '/' + repository).build());
+            registerRepository(repository, FsRepository.TYPE, true, repositorySettings(repository));
 
             final String originalIndex = "logs_" + suffix;
             createIndex(originalIndex, Settings.builder()
-                .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, randomIntBetween(3, 5))
+                .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, randomIntBetween(1, 3))
                 .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
                 .build());
             indexDocs(originalIndex, numberOfDocs);
@@ -81,6 +77,12 @@ private void executeMountAndRecoversCorrectlyTestCase(Storage storage, long numb
 
         ensureGreen(index);
         assertHitCount(index, equalTo(numberOfDocs));
+
+        if (CLUSTER_TYPE.equals(ClusterType.UPGRADED)) {
+            deleteIndex(index);
+            deleteSnapshot(repository, snapshot);
+            deleteRepository(repository);
+        }
     }
 
     public void testBlobStoreCacheWithFullCopyInMixedVersions() throws Exception {
@@ -107,8 +109,7 @@ private void executeBlobCacheCreationTestCase(Storage storage, long numberOfDocs
         final String repository = "repository_" + suffix;
 
         if (CLUSTER_TYPE.equals(ClusterType.OLD)) {
-            registerRepository(repository, FsRepository.TYPE, true,
-                Settings.builder().put("location", System.getProperty("tests.path.repo") + '/' + repository).build());
+            registerRepository(repository, FsRepository.TYPE, true, repositorySettings(repository));
 
         } else if (CLUSTER_TYPE.equals(ClusterType.MIXED)) {
             final int numberOfNodes = 3;
@@ -135,7 +136,7 @@ private void executeBlobCacheCreationTestCase(Storage storage, long numberOfDocs
 
             final String firstIndex = "first_index_" + suffix;
             createIndex(firstIndex, Settings.builder()
-                .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, randomIntBetween(3, 5))
+                .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, randomIntBetween(1, 3))
                 .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
                 .build());
             indexDocs(firstIndex, numberOfDocs);
@@ -287,8 +288,11 @@ private static void mountSnapshot(
         Settings indexSettings
     ) throws IOException {
         final Request request = new Request(HttpPost.METHOD_NAME, "/_snapshot/" + repositoryName + '/' + snapshotName + "/_mount");
-        request.addParameter("storage", storage.storageName());
-        request.addParameter("wait_for_completion", "true");
+        if (UPGRADE_FROM_VERSION.onOrAfter(Version.V_7_12_0)) {
+            request.addParameter("storage", storage.storageName());
+        } else {
+            assertThat("Parameter 'storage' was introduced in 7.12.0 with " + Storage.SHARED_CACHE, storage, equalTo(Storage.FULL_COPY));
+        }
         request.setJsonEntity("{" +
             "  \"index\": \"" + indexName + "\"," +
             "  \"renamed_index\": \"" + renamedIndex + "\"," +
@@ -311,4 +315,12 @@ private static void assertHitCount(String indexName, Matcher<Long> countMatcher)
         assertThat(((Number) extractValue("count", responseAsMap)).longValue(), countMatcher);
         assertThat(((Number) extractValue("_shards.failed", responseAsMap)).intValue(), equalTo(0));
     }
+
+    private static Settings repositorySettings(String repository) {
+        final String pathRepo = System.getProperty("tests.path.searchable.snapshots.repo");
+        assertThat("Searchable snapshots repository path is null", pathRepo, notNullValue());
+        return Settings.builder()
+            .put("location", pathRepo + '/' + repository)
+            .build();
+    }
 }
