From 6046f172c89eeb2180ead7784adc4595e6289260 Mon Sep 17 00:00:00 2001 From: David Pilato Date: Thu, 31 Jul 2014 00:11:53 +0200 Subject: [PATCH] listBlobsByPrefix uses a wrong path When listing existing blobs for an azure repository, `path` to look at is incorrectly computed which leads to 404 errors. Closes #26. (cherry picked from commit 656fadc) --- .../cloud/azure/AzureStorageService.java | 2 +- .../cloud/azure/AzureStorageServiceImpl.java | 17 ++- .../blobstore/AbstractAzureBlobContainer.java | 16 +-- .../azure/AzureSnapshotRestoreITest.java | 122 ++++++++++++++---- .../azure/AzureStorageServiceMock.java | 2 +- 5 files changed, 113 insertions(+), 46 deletions(-) diff --git a/src/main/java/org/elasticsearch/cloud/azure/AzureStorageService.java b/src/main/java/org/elasticsearch/cloud/azure/AzureStorageService.java index f346348ed8874..700fc6ab4ac6d 100644 --- a/src/main/java/org/elasticsearch/cloud/azure/AzureStorageService.java +++ b/src/main/java/org/elasticsearch/cloud/azure/AzureStorageService.java @@ -57,7 +57,7 @@ static public final class Fields { InputStream getInputStream(String container, String blob) throws ServiceException; - ImmutableMap listBlobsByPrefix(String container, String prefix) throws URISyntaxException, StorageException, ServiceException; + ImmutableMap listBlobsByPrefix(String container, String keyPath, String prefix) throws URISyntaxException, StorageException, ServiceException; void putObject(String container, String blob, InputStream is, long length) throws URISyntaxException, StorageException, IOException; diff --git a/src/main/java/org/elasticsearch/cloud/azure/AzureStorageServiceImpl.java b/src/main/java/org/elasticsearch/cloud/azure/AzureStorageServiceImpl.java index c396aaae679fa..5c617798eff03 100644 --- a/src/main/java/org/elasticsearch/cloud/azure/AzureStorageServiceImpl.java +++ b/src/main/java/org/elasticsearch/cloud/azure/AzureStorageServiceImpl.java @@ -113,7 +113,14 @@ public boolean doesContainerExist(String container) { @Override public void removeContainer(String container) throws URISyntaxException, StorageException { CloudBlobContainer blob_container = client.getContainerReference(container); - blob_container.delete(); + // TODO Should we set some timeout and retry options? + /* + BlobRequestOptions options = new BlobRequestOptions(); + options.setTimeoutIntervalInMs(1000); + options.setRetryPolicyFactory(new RetryNoRetry()); + blob_container.deleteIfExists(options, null); + */ + blob_container.deleteIfExists(); } @Override @@ -188,13 +195,13 @@ public InputStream getInputStream(String container, String blob) throws ServiceE } @Override - public ImmutableMap listBlobsByPrefix(String container, String prefix) throws URISyntaxException, StorageException, ServiceException { - logger.debug("listBlobsByPrefix container [{}], prefix [{}]", container, prefix); + public ImmutableMap listBlobsByPrefix(String container, String keyPath, String prefix) throws URISyntaxException, StorageException, ServiceException { + logger.debug("listBlobsByPrefix container [{}], keyPath [{}], prefix [{}]", container, keyPath, prefix); ImmutableMap.Builder blobsBuilder = ImmutableMap.builder(); CloudBlobContainer blob_container = client.getContainerReference(container); if (blob_container.exists()) { - Iterable blobs = blob_container.listBlobs(prefix); + Iterable blobs = blob_container.listBlobs(keyPath + prefix); for (ListBlobItem blob : blobs) { URI uri = blob.getUri(); if (logger.isTraceEnabled()) { @@ -202,7 +209,7 @@ public ImmutableMap listBlobsByPrefix(String container, St } String blobpath = uri.getPath().substring(container.length() + 1); BlobProperties properties = service.getBlobProperties(container, blobpath).getProperties(); - String name = uri.getPath().substring(prefix.length()); + String name = blobpath.substring(keyPath.length() + 1); if (logger.isTraceEnabled()) { logger.trace("blob url [{}], name [{}], size [{}]", uri, name, properties.getContentLength()); } diff --git a/src/main/java/org/elasticsearch/cloud/azure/blobstore/AbstractAzureBlobContainer.java b/src/main/java/org/elasticsearch/cloud/azure/blobstore/AbstractAzureBlobContainer.java index cc94510c36b2b..38e9bc95180a0 100644 --- a/src/main/java/org/elasticsearch/cloud/azure/blobstore/AbstractAzureBlobContainer.java +++ b/src/main/java/org/elasticsearch/cloud/azure/blobstore/AbstractAzureBlobContainer.java @@ -111,24 +111,18 @@ public void run() { } @Override - public ImmutableMap listBlobsByPrefix(@Nullable String blobNamePrefix) throws IOException { - final String prefix; - if (blobNamePrefix != null) { - prefix = buildKey(blobNamePrefix); - } else { - prefix = keyPath; - } + public ImmutableMap listBlobsByPrefix(@Nullable String prefix) throws IOException { try { - return blobStore.client().listBlobsByPrefix(blobStore.container(), prefix); + return blobStore.client().listBlobsByPrefix(blobStore.container(), keyPath, prefix); } catch (URISyntaxException e) { - logger.warn("can not access [{}] in container {{}}: {}", blobNamePrefix, blobStore.container(), e.getMessage()); + logger.warn("can not access [{}] in container {{}}: {}", prefix, blobStore.container(), e.getMessage()); throw new IOException(e); } catch (StorageException e) { - logger.warn("can not access [{}] in container {{}}: {}", blobNamePrefix, blobStore.container(), e.getMessage()); + logger.warn("can not access [{}] in container {{}}: {}", prefix, blobStore.container(), e.getMessage()); throw new IOException(e); } catch (ServiceException e) { - logger.warn("can not access [{}] in container {{}}: {}", blobNamePrefix, blobStore.container(), e.getMessage()); + logger.warn("can not access [{}] in container {{}}: {}", prefix, blobStore.container(), e.getMessage()); throw new IOException(e); } } diff --git a/src/test/java/org/elasticsearch/repositories/azure/AzureSnapshotRestoreITest.java b/src/test/java/org/elasticsearch/repositories/azure/AzureSnapshotRestoreITest.java index 18173cebf6d5b..86c1c3ef536be 100644 --- a/src/test/java/org/elasticsearch/repositories/azure/AzureSnapshotRestoreITest.java +++ b/src/test/java/org/elasticsearch/repositories/azure/AzureSnapshotRestoreITest.java @@ -27,6 +27,7 @@ import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotResponse; import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotResponse; import org.elasticsearch.client.Client; +import org.elasticsearch.client.ClusterAdminClient; import org.elasticsearch.cloud.azure.AbstractAzureTest; import org.elasticsearch.cloud.azure.AzureStorageService; import org.elasticsearch.cluster.ClusterState; @@ -55,14 +56,18 @@ @AbstractAzureTest.AzureTest @ElasticsearchIntegrationTest.ClusterScope( scope = ElasticsearchIntegrationTest.Scope.SUITE, - numDataNodes = 2, + numDataNodes = 1, transportClientRatio = 0.0) public class AzureSnapshotRestoreITest extends AbstractAzureTest { - private final String basePath; + private String getRepositoryPath() { + String testName = "/snapshot-itest/repo-".concat("" + randomIntBetween(1, 1000)); + return testName.contains(" ") ? Strings.split(testName, " ")[0] : testName; + } - public AzureSnapshotRestoreITest() { - basePath = "/snapshot-itest/repo-" + randomInt(); + private String getContainerName() { + String testName = "it-".concat(Strings.toUnderscoreCase(getTestName()).replaceAll("_", "-")); + return testName.contains(" ") ? Strings.split(testName, " ")[0] : testName; } @Override @@ -78,23 +83,29 @@ public Settings indexSettings() { @Before public final void wipeBefore() throws StorageException, ServiceException, URISyntaxException { wipeRepositories(); - cleanRepositoryFiles(basePath); + cleanRepositoryFiles( + getContainerName(), + getContainerName().concat("-1"), + getContainerName().concat("-2")); } @After public final void wipeAfter() throws StorageException, ServiceException, URISyntaxException { wipeRepositories(); - cleanRepositoryFiles(basePath); + cleanRepositoryFiles( + getContainerName(), + getContainerName().concat("-1"), + getContainerName().concat("-2")); } @Test public void testSimpleWorkflow() { Client client = client(); - logger.info("--> creating azure repository with path [{}]", basePath); + logger.info("--> creating azure repository with path [{}]", getRepositoryPath()); PutRepositoryResponse putRepositoryResponse = client.admin().cluster().preparePutRepository("test-repo") .setType("azure").setSettings(ImmutableSettings.settingsBuilder() - .put(AzureStorageService.Fields.CONTAINER, "elasticsearch-integration") - .put(AzureStorageService.Fields.BASE_PATH, basePath) + .put(AzureStorageService.Fields.CONTAINER, getContainerName()) + .put(AzureStorageService.Fields.BASE_PATH, getRepositoryPath()) .put(AzureStorageService.Fields.CHUNK_SIZE, randomIntBetween(1000, 10000)) ).get(); assertThat(putRepositoryResponse.isAcknowledged(), equalTo(true)); @@ -163,18 +174,18 @@ public void testSimpleWorkflow() { @Test public void testMultipleRepositories() { Client client = client(); - logger.info("--> creating azure repository with path [{}]", basePath); + logger.info("--> creating azure repository with path [{}]", getRepositoryPath()); PutRepositoryResponse putRepositoryResponse1 = client.admin().cluster().preparePutRepository("test-repo1") .setType("azure").setSettings(ImmutableSettings.settingsBuilder() - .put(AzureStorageService.Fields.CONTAINER, "elasticsearch-integration1") - .put(AzureStorageService.Fields.BASE_PATH, basePath) + .put(AzureStorageService.Fields.CONTAINER, getContainerName().concat("-1")) + .put(AzureStorageService.Fields.BASE_PATH, getRepositoryPath()) .put(AzureStorageService.Fields.CHUNK_SIZE, randomIntBetween(1000, 10000)) ).get(); assertThat(putRepositoryResponse1.isAcknowledged(), equalTo(true)); PutRepositoryResponse putRepositoryResponse2 = client.admin().cluster().preparePutRepository("test-repo2") .setType("azure").setSettings(ImmutableSettings.settingsBuilder() - .put(AzureStorageService.Fields.CONTAINER, "elasticsearch-integration2") - .put(AzureStorageService.Fields.BASE_PATH, basePath) + .put(AzureStorageService.Fields.CONTAINER, getContainerName().concat("-2")) + .put(AzureStorageService.Fields.BASE_PATH, getRepositoryPath()) .put(AzureStorageService.Fields.CHUNK_SIZE, randomIntBetween(1000, 10000)) ).get(); assertThat(putRepositoryResponse2.isAcknowledged(), equalTo(true)); @@ -226,6 +237,65 @@ public void testMultipleRepositories() { assertThat(clusterState.getMetaData().hasIndex("test-idx-2"), equalTo(true)); } + /** + * For issue #26: https://github.com/elasticsearch/elasticsearch-cloud-azure/issues/26 + */ + @Test + public void testListBlobs_26() throws StorageException, ServiceException, URISyntaxException { + createIndex("test-idx-1", "test-idx-2", "test-idx-3"); + ensureGreen(); + + logger.info("--> indexing some data"); + for (int i = 0; i < 100; i++) { + index("test-idx-1", "doc", Integer.toString(i), "foo", "bar" + i); + index("test-idx-2", "doc", Integer.toString(i), "foo", "baz" + i); + index("test-idx-3", "doc", Integer.toString(i), "foo", "baz" + i); + } + refresh(); + + ClusterAdminClient client = client().admin().cluster(); + logger.info("--> creating azure repository without any path"); + PutRepositoryResponse putRepositoryResponse = client.preparePutRepository("test-repo").setType("azure") + .setSettings(ImmutableSettings.settingsBuilder() + .put(AzureStorageService.Fields.CONTAINER, getContainerName()) + ).get(); + assertThat(putRepositoryResponse.isAcknowledged(), equalTo(true)); + + // Get all snapshots - should be empty + assertThat(client.prepareGetSnapshots("test-repo").get().getSnapshots().size(), equalTo(0)); + + logger.info("--> snapshot"); + CreateSnapshotResponse createSnapshotResponse = client.prepareCreateSnapshot("test-repo", "test-snap").setWaitForCompletion(true).setIndices("test-idx-*").get(); + assertThat(createSnapshotResponse.getSnapshotInfo().successfulShards(), greaterThan(0)); + + // Get all snapshots - should have one + assertThat(client.prepareGetSnapshots("test-repo").get().getSnapshots().size(), equalTo(1)); + + // Clean the snapshot + client.prepareDeleteSnapshot("test-repo", "test-snap").get(); + client.prepareDeleteRepository("test-repo").get(); + + logger.info("--> creating azure repository path [{}]", getRepositoryPath()); + putRepositoryResponse = client.preparePutRepository("test-repo").setType("azure") + .setSettings(ImmutableSettings.settingsBuilder() + .put(AzureStorageService.Fields.CONTAINER, getContainerName()) + .put(AzureStorageService.Fields.BASE_PATH, getRepositoryPath()) + ).get(); + assertThat(putRepositoryResponse.isAcknowledged(), equalTo(true)); + + // Get all snapshots - should be empty + assertThat(client.prepareGetSnapshots("test-repo").get().getSnapshots().size(), equalTo(0)); + + logger.info("--> snapshot"); + createSnapshotResponse = client.prepareCreateSnapshot("test-repo", "test-snap").setWaitForCompletion(true).setIndices("test-idx-*").get(); + assertThat(createSnapshotResponse.getSnapshotInfo().successfulShards(), greaterThan(0)); + + // Get all snapshots - should have one + assertThat(client.prepareGetSnapshots("test-repo").get().getSnapshots().size(), equalTo(1)); + + + } + /** * For issue #21: https://github.com/elasticsearch/elasticsearch-cloud-azure/issues/21 */ @@ -254,7 +324,7 @@ private void checkContainerName(String container, boolean correct) { PutRepositoryResponse putRepositoryResponse = client().admin().cluster().preparePutRepository("test-repo") .setType("azure").setSettings(ImmutableSettings.settingsBuilder() .put(AzureStorageService.Fields.CONTAINER, container) - .put(AzureStorageService.Fields.BASE_PATH, basePath) + .put(AzureStorageService.Fields.BASE_PATH, getRepositoryPath()) .put(AzureStorageService.Fields.CHUNK_SIZE, randomIntBetween(1000, 10000)) ).get(); assertThat(putRepositoryResponse.isAcknowledged(), is(correct)); @@ -273,11 +343,11 @@ private void checkContainerName(String container, boolean correct) { @Test public void testNonExistingRepo_23() { Client client = client(); - logger.info("--> creating azure repository with path [{}]", basePath); + logger.info("--> creating azure repository with path [{}]", getRepositoryPath()); PutRepositoryResponse putRepositoryResponse = client.admin().cluster().preparePutRepository("test-repo") .setType("azure").setSettings(ImmutableSettings.settingsBuilder() - .put(AzureStorageService.Fields.CONTAINER, "elasticsearch-integration") - .put(AzureStorageService.Fields.BASE_PATH, basePath) + .put(AzureStorageService.Fields.CONTAINER, getContainerName()) + .put(AzureStorageService.Fields.BASE_PATH, getRepositoryPath()) .put(AzureStorageService.Fields.CHUNK_SIZE, randomIntBetween(1000, 10000)) ).get(); assertThat(putRepositoryResponse.isAcknowledged(), equalTo(true)); @@ -309,17 +379,13 @@ public static void wipeRepositories(String... repositories) { } /** - * Purge the test container + * Purge the test containers */ - public void cleanRepositoryFiles(String path) throws StorageException, ServiceException, URISyntaxException { - String container = internalCluster().getInstance(Settings.class).get("repositories.azure.container", - AzureRepository.CONTAINER_DEFAULT); - logger.info("--> remove blobs in container [{}], path [{}]", container, path); + public void cleanRepositoryFiles(String... containers) throws StorageException, ServiceException, URISyntaxException { AzureStorageService client = internalCluster().getInstance(AzureStorageService.class); - - // Remove starting / if any - path = Strings.trimLeadingCharacter(path, '/'); - - client.deleteFiles(container, path); + for (String container : containers) { + logger.info("--> remove container [{}]", container); + client.removeContainer(container); + } } } diff --git a/src/test/java/org/elasticsearch/repositories/azure/AzureStorageServiceMock.java b/src/test/java/org/elasticsearch/repositories/azure/AzureStorageServiceMock.java index 689bee26b211a..63b1d12bd35f4 100644 --- a/src/test/java/org/elasticsearch/repositories/azure/AzureStorageServiceMock.java +++ b/src/test/java/org/elasticsearch/repositories/azure/AzureStorageServiceMock.java @@ -82,7 +82,7 @@ public InputStream getInputStream(String container, String blob) { } @Override - public ImmutableMap listBlobsByPrefix(String container, String prefix) { + public ImmutableMap listBlobsByPrefix(String container, String keyPath, String prefix) { ImmutableMap.Builder blobsBuilder = ImmutableMap.builder(); for (String blobName : blobs.keySet()) { if (Strings.startsWithIgnoreCase(blobName, prefix)) {