From 9e14ffa8be205d88775f9535c9a5f6c9521039ba Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Wed, 28 Aug 2019 16:13:31 +0200 Subject: [PATCH] Few clean ups in ESBlobStoreRepositoryIntegTestCase (#46068) --- ...eCloudStorageBlobStoreRepositoryTests.java | 35 ++++---- .../s3/S3BlobStoreRepositoryTests.java | 25 +++--- .../fs/FsBlobStoreRepositoryIT.java | 24 +++--- .../ESBlobStoreRepositoryIntegTestCase.java | 81 ++++++++++--------- 4 files changed, 77 insertions(+), 88 deletions(-) diff --git a/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobStoreRepositoryTests.java b/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobStoreRepositoryTests.java index 0e3ecde69c4..fa9631d1a00 100644 --- a/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobStoreRepositoryTests.java +++ b/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobStoreRepositoryTests.java @@ -25,7 +25,6 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.ByteSizeUnit; import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.plugins.Plugin; -import org.elasticsearch.repositories.Repository; import org.elasticsearch.repositories.blobstore.ESBlobStoreRepositoryIntegTestCase; import org.junit.After; @@ -34,9 +33,6 @@ import java.util.Collections; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; -import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; -import static org.hamcrest.Matchers.instanceOf; - public class GoogleCloudStorageBlobStoreRepositoryTests extends ESBlobStoreRepositoryIntegTestCase { private static final String BUCKET = "gcs-repository-test"; @@ -45,28 +41,25 @@ public class GoogleCloudStorageBlobStoreRepositoryTests extends ESBlobStoreRepos // all nodes must see the same content private static final ConcurrentMap blobs = new ConcurrentHashMap<>(); + @Override + protected String repositoryType() { + return GoogleCloudStorageRepository.TYPE; + } + + @Override + protected Settings repositorySettings() { + return Settings.builder() + .put(super.repositorySettings()) + .put("bucket", BUCKET) + .put("base_path", GoogleCloudStorageBlobStoreRepositoryTests.class.getSimpleName()) + .build(); + } + @Override protected Collection> nodePlugins() { return Collections.singletonList(MockGoogleCloudStoragePlugin.class); } - @Override - protected void createTestRepository(String name, boolean verify) { - assertAcked(client().admin().cluster().preparePutRepository(name) - .setType(GoogleCloudStorageRepository.TYPE) - .setVerify(verify) - .setSettings(Settings.builder() - .put("bucket", BUCKET) - .put("base_path", GoogleCloudStorageBlobStoreRepositoryTests.class.getSimpleName()) - .put("compress", randomBoolean()) - .put("chunk_size", randomIntBetween(100, 1000), ByteSizeUnit.BYTES))); - } - - @Override - protected void afterCreationCheck(Repository repository) { - assertThat(repository, instanceOf(GoogleCloudStorageRepository.class)); - } - @After public void wipeRepository() { blobs.clear(); diff --git a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryTests.java b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryTests.java index d7aaffadf51..efb07759c2b 100644 --- a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryTests.java +++ b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryTests.java @@ -40,9 +40,6 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; -import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; -import static org.hamcrest.Matchers.instanceOf; - public class S3BlobStoreRepositoryTests extends ESBlobStoreRepositoryIntegTestCase { private static final ConcurrentMap blobs = new ConcurrentHashMap<>(); @@ -71,21 +68,19 @@ public class S3BlobStoreRepositoryTests extends ESBlobStoreRepositoryIntegTestCa } @Override - protected void createTestRepository(final String name, boolean verify) { - assertAcked(client().admin().cluster().preparePutRepository(name) - .setType(S3Repository.TYPE) - .setVerify(verify) - .setSettings(Settings.builder() - .put(S3Repository.BUCKET_SETTING.getKey(), bucket) - .put(S3Repository.BUFFER_SIZE_SETTING.getKey(), bufferSize) - .put(S3Repository.SERVER_SIDE_ENCRYPTION_SETTING.getKey(), serverSideEncryption) - .put(S3Repository.CANNED_ACL_SETTING.getKey(), cannedACL) - .put(S3Repository.STORAGE_CLASS_SETTING.getKey(), storageClass))); + protected String repositoryType() { + return S3Repository.TYPE; } @Override - protected void afterCreationCheck(Repository repository) { - assertThat(repository, instanceOf(S3Repository.class)); + protected Settings repositorySettings() { + return Settings.builder() + .put(S3Repository.BUCKET_SETTING.getKey(), bucket) + .put(S3Repository.BUFFER_SIZE_SETTING.getKey(), bufferSize) + .put(S3Repository.SERVER_SIDE_ENCRYPTION_SETTING.getKey(), serverSideEncryption) + .put(S3Repository.CANNED_ACL_SETTING.getKey(), cannedACL) + .put(S3Repository.STORAGE_CLASS_SETTING.getKey(), storageClass) + .build(); } @Override diff --git a/server/src/test/java/org/elasticsearch/repositories/fs/FsBlobStoreRepositoryIT.java b/server/src/test/java/org/elasticsearch/repositories/fs/FsBlobStoreRepositoryIT.java index dd4ca7bfd20..b89635af97d 100644 --- a/server/src/test/java/org/elasticsearch/repositories/fs/FsBlobStoreRepositoryIT.java +++ b/server/src/test/java/org/elasticsearch/repositories/fs/FsBlobStoreRepositoryIT.java @@ -22,7 +22,6 @@ import org.elasticsearch.ElasticsearchException; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.ByteSizeUnit; import org.elasticsearch.core.internal.io.IOUtils; -import org.elasticsearch.repositories.Repository; import org.elasticsearch.repositories.blobstore.ESBlobStoreRepositoryIntegTestCase; import java.io.IOException; @@ -37,23 +36,22 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitC import static org.hamcrest.Matchers.instanceOf; public class FsBlobStoreRepositoryIT extends ESBlobStoreRepositoryIntegTestCase { + @Override - protected void createTestRepository(String name, boolean verify) { - assertAcked(client().admin().cluster().preparePutRepository(name) - .setVerify(verify) - .setType("fs").setSettings(Settings.builder() - .put("location", randomRepoPath()) - .put("compress", randomBoolean()) - .put("chunk_size", randomIntBetween(100, 1000), ByteSizeUnit.BYTES))); + protected String repositoryType() { + return FsRepository.TYPE; } @Override - protected void afterCreationCheck(Repository repository) { - assertThat(repository, instanceOf(FsRepository.class)); + protected Settings repositorySettings() { + return Settings.builder() + .put(super.repositorySettings()) + .put("location", randomRepoPath()) + .build(); } public void testMissingDirectoriesNotCreatedInReadonlyRepository() throws IOException, ExecutionException, InterruptedException { - final String repoName = randomAsciiName(); + final String repoName = randomName(); final Path repoPath = randomRepoPath(); logger.info("--> creating repository {} at {}", repoName, repoPath); @@ -63,13 +61,13 @@ public class FsBlobStoreRepositoryIT extends ESBlobStoreRepositoryIntegTestCase .put("compress", randomBoolean()) .put("chunk_size", randomIntBetween(100, 1000), ByteSizeUnit.BYTES))); - String indexName = randomAsciiName(); + final String indexName = randomName(); int docCount = iterations(10, 1000); logger.info("--> create random index {} with {} records", indexName, docCount); addRandomDocuments(indexName, docCount); assertHitCount(client().prepareSearch(indexName).setSize(0).get(), docCount); - final String snapshotName = randomAsciiName(); + final String snapshotName = randomName(); logger.info("--> create snapshot {}:{}", repoName, snapshotName); assertSuccessfulSnapshot(client().admin().cluster().prepareCreateSnapshot(repoName, snapshotName) .setWaitForCompletion(true).setIndices(indexName)); diff --git a/test/framework/src/main/java/org/elasticsearch/repositories/blobstore/ESBlobStoreRepositoryIntegTestCase.java b/test/framework/src/main/java/org/elasticsearch/repositories/blobstore/ESBlobStoreRepositoryIntegTestCase.java index cc835b24535..6a11e655def 100644 --- a/test/framework/src/main/java/org/elasticsearch/repositories/blobstore/ESBlobStoreRepositoryIntegTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/repositories/blobstore/ESBlobStoreRepositoryIntegTestCase.java @@ -26,9 +26,12 @@ import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotR import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.client.Client; import org.elasticsearch.common.blobstore.BlobContainer; +import org.elasticsearch.common.blobstore.BlobStore; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.unit.ByteSizeUnit; +import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.repositories.IndexId; import org.elasticsearch.repositories.RepositoriesService; -import org.elasticsearch.repositories.Repository; import org.elasticsearch.repositories.RepositoryData; import org.elasticsearch.snapshots.SnapshotMissingException; import org.elasticsearch.snapshots.SnapshotRestoreException; @@ -47,42 +50,50 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcke import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; /** - * Basic integration tests for blob-based repository validation. + * Integration tests for {@link BlobStoreRepository} implementations. */ public abstract class ESBlobStoreRepositoryIntegTestCase extends ESIntegTestCase { - protected abstract void createTestRepository(String name, boolean verify); - - protected void afterCreationCheck(Repository repository) { + protected abstract String repositoryType(); + protected Settings repositorySettings() { + final Settings.Builder settings = Settings.builder(); + settings.put("compress", randomBoolean()); + if (randomBoolean()) { + long size = 1 << randomIntBetween(7, 10); + settings.put("chunk_size", new ByteSizeValue(size, randomFrom(ByteSizeUnit.BYTES, ByteSizeUnit.KB))); + } + return settings.build(); } - protected void createAndCheckTestRepository(String name) { + protected final String createRepository(final String name) { final boolean verify = randomBoolean(); - createTestRepository(name, verify); - final Iterable repositoriesServices = - internalCluster().getDataOrMasterNodeInstances(RepositoriesService.class); + logger.debug("--> creating repository [name: {}, verify: {}]", name, verify); + assertAcked(client().admin().cluster().preparePutRepository(name) + .setType(repositoryType()) + .setVerify(verify) + .setSettings(repositorySettings())); - for (RepositoriesService repositoriesService : repositoriesServices) { - final BlobStoreRepository repository = (BlobStoreRepository) repositoriesService.repository(name); - - afterCreationCheck(repository); - assertThat("blob store has to be lazy initialized", - repository.getBlobStore(), verify ? is(notNullValue()) : is(nullValue())); - } + internalCluster().getDataOrMasterNodeInstances(RepositoriesService.class).forEach(repositories -> { + assertThat(repositories.repository(name), notNullValue()); + assertThat(repositories.repository(name), instanceOf(BlobStoreRepository.class)); + assertThat(repositories.repository(name).isReadOnly(), is(false)); + BlobStore blobStore = ((BlobStoreRepository) repositories.repository(name)).getBlobStore(); + assertThat("blob store has to be lazy initialized", blobStore, verify ? is(notNullValue()) : is(nullValue())); + }); + return name; } public void testSnapshotAndRestore() throws Exception { - final String repoName = randomAsciiName(); - logger.info("--> creating repository {}", repoName); - createAndCheckTestRepository(repoName); + final String repoName = createRepository(randomName()); int indexCount = randomIntBetween(1, 5); int[] docCounts = new int[indexCount]; String[] indexNames = generateRandomNames(indexCount); @@ -93,7 +104,7 @@ public abstract class ESBlobStoreRepositoryIntegTestCase extends ESIntegTestCase assertHitCount(client().prepareSearch(indexNames[i]).setSize(0).get(), docCounts[i]); } - final String snapshotName = randomAsciiName(); + final String snapshotName = randomName(); logger.info("--> create snapshot {}:{}", repoName, snapshotName); assertSuccessfulSnapshot(client().admin().cluster().prepareCreateSnapshot(repoName, snapshotName) .setWaitForCompletion(true).setIndices(indexNames)); @@ -153,13 +164,11 @@ public abstract class ESBlobStoreRepositoryIntegTestCase extends ESIntegTestCase } public void testMultipleSnapshotAndRollback() throws Exception { - String repoName = randomAsciiName(); - logger.info("--> creating repository {}", repoName); - createAndCheckTestRepository(repoName); + final String repoName = createRepository(randomName()); int iterationCount = randomIntBetween(2, 5); int[] docCounts = new int[iterationCount]; - String indexName = randomAsciiName(); - String snapshotName = randomAsciiName(); + String indexName = randomName(); + String snapshotName = randomName(); assertAcked(client().admin().indices().prepareCreate(indexName).get()); for (int i = 0; i < iterationCount; i++) { if (randomBoolean() && i > 0) { // don't delete on the first iteration @@ -210,12 +219,8 @@ public abstract class ESBlobStoreRepositoryIntegTestCase extends ESIntegTestCase } public void testIndicesDeletedFromRepository() throws Exception { + final String repoName = createRepository("test-repo"); Client client = client(); - - logger.info("--> creating repository"); - final String repoName = "test-repo"; - createAndCheckTestRepository(repoName); - createIndex("test-idx-1", "test-idx-2", "test-idx-3"); ensureGreen(); @@ -280,41 +285,39 @@ public abstract class ESBlobStoreRepositoryIntegTestCase extends ESIntegTestCase indexRandom(true, indexRequestBuilders); } - protected String[] generateRandomNames(int num) { + private String[] generateRandomNames(int num) { Set names = new HashSet<>(); for (int i = 0; i < num; i++) { String name; do { - name = randomAsciiName(); + name = randomName(); } while (names.contains(name)); names.add(name); } return names.toArray(new String[num]); } - public static CreateSnapshotResponse assertSuccessfulSnapshot(CreateSnapshotRequestBuilder requestBuilder) { + protected static void assertSuccessfulSnapshot(CreateSnapshotRequestBuilder requestBuilder) { CreateSnapshotResponse response = requestBuilder.get(); assertSuccessfulSnapshot(response); - return response; } - public static void assertSuccessfulSnapshot(CreateSnapshotResponse response) { + private static void assertSuccessfulSnapshot(CreateSnapshotResponse response) { assertThat(response.getSnapshotInfo().successfulShards(), greaterThan(0)); assertThat(response.getSnapshotInfo().successfulShards(), equalTo(response.getSnapshotInfo().totalShards())); } - public static RestoreSnapshotResponse assertSuccessfulRestore(RestoreSnapshotRequestBuilder requestBuilder) { + private static void assertSuccessfulRestore(RestoreSnapshotRequestBuilder requestBuilder) { RestoreSnapshotResponse response = requestBuilder.get(); assertSuccessfulRestore(response); - return response; } - public static void assertSuccessfulRestore(RestoreSnapshotResponse response) { + private static void assertSuccessfulRestore(RestoreSnapshotResponse response) { assertThat(response.getRestoreInfo().successfulShards(), greaterThan(0)); assertThat(response.getRestoreInfo().successfulShards(), equalTo(response.getRestoreInfo().totalShards())); } - public static String randomAsciiName() { + protected static String randomName() { return randomAlphaOfLength(randomIntBetween(1, 10)).toLowerCase(Locale.ROOT); } }