Failure when creating a container should raise an exception

When you remove a container from Azure console, even if Azure told you that it has been done, it appears to be an asynchronous deletion so you can hit error like `can not initialize container [elasticsearch-snapshots]: [The specified container is being deleted. Try operation later.]` but this error does not fail the repository creation...

Related to #54.

(cherry picked from commit e483304)
(cherry picked from commit 520444c)

src/main/java/org/elasticsearch/cloud/azure/AzureStorageServiceImpl.java

@@ -42,6 +42,7 @@ import org.elasticsearch.common.component.AbstractLifecycleComponent;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.settings.SettingsFilter;
+import org.elasticsearch.repositories.RepositoryException;
 
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -125,9 +126,14 @@ public class AzureStorageServiceImpl extends AbstractLifecycleComponent<AzureSto
 
     @Override
     public void createContainer(String container) throws URISyntaxException, StorageException {
+        try {
             CloudBlobContainer blob_container = client.getContainerReference(container);
             logger.trace("creating container [{}]", container);
             blob_container.createIfNotExist();
+        } catch (IllegalArgumentException e) {
+            logger.trace("fails creating container [{}]", container, e.getMessage());
+            throw new RepositoryException(container, e.getMessage());
+        }
     }
 
     @Override
@@ -175,18 +181,20 @@ public class AzureStorageServiceImpl extends AbstractLifecycleComponent<AzureSto
 
     @Override
     public InputStream getInputStream(String container, String blob) throws ServiceException {
+        logger.trace("reading container [{}], blob [{}]", container, blob);
         GetBlobResult blobResult = service.getBlob(container, blob);
         return blobResult.getContentStream();
     }
 
     @Override
     public OutputStream getOutputStream(String container, String blob) throws URISyntaxException, StorageException {
+        logger.trace("writing container [{}], blob [{}]", container, blob);
         return client.getContainerReference(container).getBlockBlobReference(blob).openOutputStream();
     }
 
     @Override
     public ImmutableMap<String, BlobMetaData> listBlobsByPrefix(String container, String keyPath, String prefix) throws URISyntaxException, StorageException, ServiceException {
-        logger.debug("listBlobsByPrefix container [{}], keyPath [{}], prefix [{}]", container, keyPath, prefix);
+        logger.debug("listing container [{}], keyPath [{}], prefix [{}]", container, keyPath, prefix);
         ImmutableMap.Builder<String, BlobMetaData> blobsBuilder = ImmutableMap.builder();
 
         CloudBlobContainer blob_container = client.getContainerReference(container);

src/main/java/org/elasticsearch/cloud/azure/blobstore/AzureBlobContainer.java

@@ -28,6 +28,7 @@ import org.elasticsearch.common.blobstore.support.AbstractBlobContainer;
 import org.elasticsearch.common.collect.ImmutableMap;
 import org.elasticsearch.common.logging.ESLogger;
 import org.elasticsearch.common.logging.Loggers;
+import org.elasticsearch.repositories.RepositoryException;
 
 import java.io.FileNotFoundException;
 import java.io.IOException;
@@ -45,8 +46,9 @@ public class AzureBlobContainer extends AbstractBlobContainer {
     protected final AzureBlobStore blobStore;
 
     protected final String keyPath;
+    protected final String repositoryName;
 
-    public AzureBlobContainer(BlobPath path, AzureBlobStore blobStore) {
+    public AzureBlobContainer(String repositoryName, BlobPath path, AzureBlobStore blobStore) {
         super(path);
         this.blobStore = blobStore;
         String keyPath = path.buildAsString("/");
@@ -54,6 +56,7 @@ public class AzureBlobContainer extends AbstractBlobContainer {
             keyPath = keyPath + "/";
         }
         this.keyPath = keyPath;
+        this.repositoryName = repositoryName;
     }
 
     @Override
@@ -89,6 +92,8 @@ public class AzureBlobContainer extends AbstractBlobContainer {
             throw new IOException(e);
         } catch (URISyntaxException e) {
             throw new IOException(e);
+        } catch (IllegalArgumentException e) {
+            throw new RepositoryException(repositoryName, e.getMessage());
         }
     }
 

src/main/java/org/elasticsearch/cloud/azure/blobstore/AzureBlobStore.java

@@ -26,7 +26,11 @@ import org.elasticsearch.common.blobstore.BlobContainer;
 import org.elasticsearch.common.blobstore.BlobPath;
 import org.elasticsearch.common.blobstore.BlobStore;
 import org.elasticsearch.common.component.AbstractComponent;
+import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.repositories.RepositoryName;
+import org.elasticsearch.repositories.RepositorySettings;
+import org.elasticsearch.repositories.azure.AzureRepository;
 
 import java.net.URISyntaxException;
 
@@ -38,15 +42,16 @@ public class AzureBlobStore extends AbstractComponent implements BlobStore {
     private final AzureStorageService client;
 
     private final String container;
+    private final String repositoryName;
 
-    public AzureBlobStore(Settings settings, AzureStorageService client, String container) throws URISyntaxException, StorageException {
+    @Inject
+    public AzureBlobStore(RepositoryName name, Settings settings, RepositorySettings repositorySettings,
+                          AzureStorageService client) throws URISyntaxException, StorageException {
         super(settings);
         this.client = client;
-        this.container = container;
+        this.container = repositorySettings.settings().get(AzureStorageService.Fields.CONTAINER,
-
+                componentSettings.get(AzureStorageService.Fields.CONTAINER, AzureRepository.CONTAINER_DEFAULT));
-        if (!client.doesContainerExist(container)) {
+        this.repositoryName = name.getName();
-            client.createContainer(container);
-        }
     }
 
     @Override
@@ -64,7 +69,7 @@ public class AzureBlobStore extends AbstractComponent implements BlobStore {
 
     @Override
     public BlobContainer blobContainer(BlobPath path) {
-        return new AzureBlobContainer(path, this);
+        return new AzureBlobContainer(repositoryName, path, this);
     }
 
     @Override

src/main/java/org/elasticsearch/repositories/azure/AzureRepository.java

@@ -22,16 +22,21 @@ package org.elasticsearch.repositories.azure;
 import com.microsoft.windowsazure.services.core.storage.StorageException;
 import org.elasticsearch.cloud.azure.AzureStorageService;
 import org.elasticsearch.cloud.azure.blobstore.AzureBlobStore;
+import org.elasticsearch.cluster.metadata.MetaData;
+import org.elasticsearch.cluster.metadata.SnapshotId;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.blobstore.BlobPath;
 import org.elasticsearch.common.blobstore.BlobStore;
+import org.elasticsearch.common.collect.ImmutableList;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.unit.ByteSizeUnit;
 import org.elasticsearch.common.unit.ByteSizeValue;
 import org.elasticsearch.index.snapshots.IndexShardRepository;
 import org.elasticsearch.repositories.RepositoryName;
 import org.elasticsearch.repositories.RepositorySettings;
+import org.elasticsearch.repositories.RepositoryVerificationException;
 import org.elasticsearch.repositories.blobstore.BlobStoreRepository;
+import org.elasticsearch.snapshots.SnapshotCreationException;
 
 import java.io.IOException;
 import java.net.URISyntaxException;
@@ -60,23 +65,16 @@ public class AzureRepository extends BlobStoreRepository {
 
     private boolean compress;
 
-    /**
-     * Constructs new shared file system repository
-     *
-     * @param name                 repository name
-     * @param repositorySettings   repository settings
-     * @param indexShardRepository index shard repository
-     * @param azureStorageService  Azure Storage service
-     * @throws java.io.IOException
-     */
     @Inject
-    public AzureRepository(RepositoryName name, RepositorySettings repositorySettings, IndexShardRepository indexShardRepository, AzureStorageService azureStorageService) throws IOException, URISyntaxException, StorageException {
+    public AzureRepository(RepositoryName name, RepositorySettings repositorySettings,
+                           IndexShardRepository indexShardRepository,
+                           AzureBlobStore azureBlobStore) throws IOException, URISyntaxException, StorageException {
         super(name.getName(), repositorySettings, indexShardRepository);
 
         String container = repositorySettings.settings().get(AzureStorageService.Fields.CONTAINER,
                 componentSettings.get(AzureStorageService.Fields.CONTAINER, CONTAINER_DEFAULT));
 
-        this.blobStore = new AzureBlobStore(settings, azureStorageService, container);
+        this.blobStore = azureBlobStore;
         this.chunkSize = repositorySettings.settings().getAsBytesSize(AzureStorageService.Fields.CHUNK_SIZE,
                 componentSettings.getAsBytesSize(AzureStorageService.Fields.CHUNK_SIZE, new ByteSizeValue(64, ByteSizeUnit.MB)));
 
@@ -133,5 +131,37 @@ public class AzureRepository extends BlobStoreRepository {
         return chunkSize;
     }
 
+    @Override
+    public void initializeSnapshot(SnapshotId snapshotId, ImmutableList<String> indices, MetaData metaData) {
+        try {
+            if (!blobStore.client().doesContainerExist(blobStore.container())) {
+                logger.debug("container [{}] does not exist. Creating...", blobStore.container());
+                blobStore.client().createContainer(blobStore.container());
+            }
+            super.initializeSnapshot(snapshotId, indices, metaData);
+        } catch (StorageException e) {
+            logger.warn("can not initialize container [{}]: [{}]", blobStore.container(), e.getMessage());
+            throw new SnapshotCreationException(snapshotId, e);
+        } catch (URISyntaxException e) {
+            logger.warn("can not initialize container [{}]: [{}]", blobStore.container(), e.getMessage());
+            throw new SnapshotCreationException(snapshotId, e);
+        }
+    }
 
+    @Override
+    public String startVerification() {
+        try {
+            if (!blobStore.client().doesContainerExist(blobStore.container())) {
+                logger.debug("container [{}] does not exist. Creating...", blobStore.container());
+                blobStore.client().createContainer(blobStore.container());
+            }
+            return super.startVerification();
+        } catch (StorageException e) {
+            logger.warn("can not initialize container [{}]: [{}]", blobStore.container(), e.getMessage());
+            throw new RepositoryVerificationException(repositoryName, "can not initialize container " + blobStore.container(), e);
+        } catch (URISyntaxException e) {
+            logger.warn("can not initialize container [{}]: [{}]", blobStore.container(), e.getMessage());
+            throw new RepositoryVerificationException(repositoryName, "can not initialize container " + blobStore.container(), e);
+        }
+    }
 }

src/test/java/org/elasticsearch/repositories/azure/AzureSnapshotRestoreITest.java

@@ -31,10 +31,12 @@ import org.elasticsearch.cloud.azure.AbstractAzureTest;
 import org.elasticsearch.cloud.azure.AzureStorageService;
 import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.common.Strings;
+import org.elasticsearch.common.base.Predicate;
 import org.elasticsearch.common.settings.ImmutableSettings;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.repositories.RepositoryException;
 import org.elasticsearch.repositories.RepositoryMissingException;
+import org.elasticsearch.repositories.RepositoryVerificationException;
 import org.elasticsearch.snapshots.SnapshotMissingException;
 import org.elasticsearch.snapshots.SnapshotState;
 import org.elasticsearch.test.ElasticsearchIntegrationTest;
@@ -44,6 +46,7 @@ import org.junit.Before;
 import org.junit.Test;
 
 import java.net.URISyntaxException;
+import java.util.concurrent.TimeUnit;
 
 import static org.hamcrest.Matchers.*;
 
@@ -388,6 +391,47 @@ public class AzureSnapshotRestoreITest extends AbstractAzureTest {
         }
     }
 
+    /**
+     * When a user remove a container you can not immediately create it again.
+     */
+    @Test
+    public void testRemoveAndCreateContainer() throws URISyntaxException, StorageException, InterruptedException {
+        final String container = getContainerName();
+        final AzureStorageService storageService = internalCluster().getInstance(AzureStorageService.class);
+
+        // It could happen that we run this test really close to a previous one
+        // so we might need some time to be able to create the container
+        assertThat(awaitBusy(new Predicate<Object>() {
+            public boolean apply(Object obj) {
+                try {
+                    storageService.createContainer(container);
+                    logger.debug(" -> container created...");
+                    return true;
+                } catch (URISyntaxException e) {
+                    // Incorrect URL. This should never happen.
+                    return false;
+                } catch (StorageException e) {
+                    // It could happen. Let's wait for a while.
+                    logger.debug(" -> container is being removed. Let's wait a bit...");
+                    return false;
+                }
+            }
+        }, 30, TimeUnit.SECONDS), equalTo(true));
+        storageService.removeContainer(container);
+
+        ClusterAdminClient client = client().admin().cluster();
+        logger.info("-->  creating azure repository while container is being removed");
+        try {
+            client.preparePutRepository("test-repo").setType("azure")
+                    .setSettings(ImmutableSettings.settingsBuilder()
+                                    .put(AzureStorageService.Fields.CONTAINER, container)
+                    ).get();
+            fail("we should get a RepositoryVerificationException");
+        } catch (RepositoryVerificationException e) {
+            // Fine we expect that
+        }
+    }
+
     /**
      * Deletes repositories, supports wildcard notation.
      */