Skip to content

Commit e483304

Browse files
dadoonetdadoonet
committed
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.
1 parent 6c98aeb commit e483304

File tree

5 files changed

+115
-23
lines changed

5 files changed

+115
-23
lines changed

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

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import org.elasticsearch.common.inject.Inject;
4343
import org.elasticsearch.common.settings.Settings;
4444
import org.elasticsearch.common.settings.SettingsFilter;
45+
import org.elasticsearch.repositories.RepositoryException;
4546

4647
import java.io.InputStream;
4748
import java.io.OutputStream;
@@ -125,9 +126,14 @@ public void removeContainer(String container) throws URISyntaxException, Storage
125126

126127
@Override
127128
public void createContainer(String container) throws URISyntaxException, StorageException {
128-
CloudBlobContainer blob_container = client.getContainerReference(container);
129-
logger.trace("creating container [{}]", container);
130-
blob_container.createIfNotExist();
129+
try {
130+
CloudBlobContainer blob_container = client.getContainerReference(container);
131+
logger.trace("creating container [{}]", container);
132+
blob_container.createIfNotExist();
133+
} catch (IllegalArgumentException e) {
134+
logger.trace("fails creating container [{}]", container, e.getMessage());
135+
throw new RepositoryException(container, e.getMessage());
136+
}
131137
}
132138

133139
@Override
@@ -175,18 +181,20 @@ public void deleteBlob(String container, String blob) throws URISyntaxException,
175181

176182
@Override
177183
public InputStream getInputStream(String container, String blob) throws ServiceException {
184+
logger.trace("reading container [{}], blob [{}]", container, blob);
178185
GetBlobResult blobResult = service.getBlob(container, blob);
179186
return blobResult.getContentStream();
180187
}
181188

182189
@Override
183190
public OutputStream getOutputStream(String container, String blob) throws URISyntaxException, StorageException {
191+
logger.trace("writing container [{}], blob [{}]", container, blob);
184192
return client.getContainerReference(container).getBlockBlobReference(blob).openOutputStream();
185193
}
186194

187195
@Override
188196
public ImmutableMap<String, BlobMetaData> listBlobsByPrefix(String container, String keyPath, String prefix) throws URISyntaxException, StorageException, ServiceException {
189-
logger.debug("listBlobsByPrefix container [{}], keyPath [{}], prefix [{}]", container, keyPath, prefix);
197+
logger.debug("listing container [{}], keyPath [{}], prefix [{}]", container, keyPath, prefix);
190198
ImmutableMap.Builder<String, BlobMetaData> blobsBuilder = ImmutableMap.builder();
191199

192200
CloudBlobContainer blob_container = client.getContainerReference(container);

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.elasticsearch.common.collect.ImmutableMap;
2929
import org.elasticsearch.common.logging.ESLogger;
3030
import org.elasticsearch.common.logging.Loggers;
31+
import org.elasticsearch.repositories.RepositoryException;
3132

3233
import java.io.FileNotFoundException;
3334
import java.io.IOException;
@@ -45,15 +46,17 @@ public class AzureBlobContainer extends AbstractBlobContainer {
4546
protected final AzureBlobStore blobStore;
4647

4748
protected final String keyPath;
49+
protected final String repositoryName;
4850

49-
public AzureBlobContainer(BlobPath path, AzureBlobStore blobStore) {
51+
public AzureBlobContainer(String repositoryName, BlobPath path, AzureBlobStore blobStore) {
5052
super(path);
5153
this.blobStore = blobStore;
5254
String keyPath = path.buildAsString("/");
5355
if (!keyPath.isEmpty()) {
5456
keyPath = keyPath + "/";
5557
}
5658
this.keyPath = keyPath;
59+
this.repositoryName = repositoryName;
5760
}
5861

5962
@Override
@@ -89,6 +92,8 @@ public OutputStream createOutput(String blobName) throws IOException {
8992
throw new IOException(e);
9093
} catch (URISyntaxException e) {
9194
throw new IOException(e);
95+
} catch (IllegalArgumentException e) {
96+
throw new RepositoryException(repositoryName, e.getMessage());
9297
}
9398
}
9499

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

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,11 @@
2626
import org.elasticsearch.common.blobstore.BlobPath;
2727
import org.elasticsearch.common.blobstore.BlobStore;
2828
import org.elasticsearch.common.component.AbstractComponent;
29+
import org.elasticsearch.common.inject.Inject;
2930
import org.elasticsearch.common.settings.Settings;
31+
import org.elasticsearch.repositories.RepositoryName;
32+
import org.elasticsearch.repositories.RepositorySettings;
33+
import org.elasticsearch.repositories.azure.AzureRepository;
3034

3135
import java.net.URISyntaxException;
3236

@@ -38,15 +42,16 @@ public class AzureBlobStore extends AbstractComponent implements BlobStore {
3842
private final AzureStorageService client;
3943

4044
private final String container;
45+
private final String repositoryName;
4146

42-
public AzureBlobStore(Settings settings, AzureStorageService client, String container) throws URISyntaxException, StorageException {
47+
@Inject
48+
public AzureBlobStore(RepositoryName name, Settings settings, RepositorySettings repositorySettings,
49+
AzureStorageService client) throws URISyntaxException, StorageException {
4350
super(settings);
4451
this.client = client;
45-
this.container = container;
46-
47-
if (!client.doesContainerExist(container)) {
48-
client.createContainer(container);
49-
}
52+
this.container = repositorySettings.settings().get(AzureStorageService.Fields.CONTAINER,
53+
componentSettings.get(AzureStorageService.Fields.CONTAINER, AzureRepository.CONTAINER_DEFAULT));
54+
this.repositoryName = name.getName();
5055
}
5156

5257
@Override
@@ -64,7 +69,7 @@ public String container() {
6469

6570
@Override
6671
public BlobContainer blobContainer(BlobPath path) {
67-
return new AzureBlobContainer(path, this);
72+
return new AzureBlobContainer(repositoryName, path, this);
6873
}
6974

7075
@Override

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

Lines changed: 41 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,16 +22,21 @@
2222
import com.microsoft.windowsazure.services.core.storage.StorageException;
2323
import org.elasticsearch.cloud.azure.AzureStorageService;
2424
import org.elasticsearch.cloud.azure.blobstore.AzureBlobStore;
25+
import org.elasticsearch.cluster.metadata.MetaData;
26+
import org.elasticsearch.cluster.metadata.SnapshotId;
2527
import org.elasticsearch.common.Strings;
2628
import org.elasticsearch.common.blobstore.BlobPath;
2729
import org.elasticsearch.common.blobstore.BlobStore;
30+
import org.elasticsearch.common.collect.ImmutableList;
2831
import org.elasticsearch.common.inject.Inject;
2932
import org.elasticsearch.common.unit.ByteSizeUnit;
3033
import org.elasticsearch.common.unit.ByteSizeValue;
3134
import org.elasticsearch.index.snapshots.IndexShardRepository;
3235
import org.elasticsearch.repositories.RepositoryName;
3336
import org.elasticsearch.repositories.RepositorySettings;
37+
import org.elasticsearch.repositories.RepositoryVerificationException;
3438
import org.elasticsearch.repositories.blobstore.BlobStoreRepository;
39+
import org.elasticsearch.snapshots.SnapshotCreationException;
3540

3641
import java.io.IOException;
3742
import java.net.URISyntaxException;
@@ -60,23 +65,16 @@ public class AzureRepository extends BlobStoreRepository {
6065

6166
private boolean compress;
6267

63-
/**
64-
* Constructs new shared file system repository
65-
*
66-
* @param name repository name
67-
* @param repositorySettings repository settings
68-
* @param indexShardRepository index shard repository
69-
* @param azureStorageService Azure Storage service
70-
* @throws java.io.IOException
71-
*/
7268
@Inject
73-
public AzureRepository(RepositoryName name, RepositorySettings repositorySettings, IndexShardRepository indexShardRepository, AzureStorageService azureStorageService) throws IOException, URISyntaxException, StorageException {
69+
public AzureRepository(RepositoryName name, RepositorySettings repositorySettings,
70+
IndexShardRepository indexShardRepository,
71+
AzureBlobStore azureBlobStore) throws IOException, URISyntaxException, StorageException {
7472
super(name.getName(), repositorySettings, indexShardRepository);
7573

7674
String container = repositorySettings.settings().get(AzureStorageService.Fields.CONTAINER,
7775
componentSettings.get(AzureStorageService.Fields.CONTAINER, CONTAINER_DEFAULT));
7876

79-
this.blobStore = new AzureBlobStore(settings, azureStorageService, container);
77+
this.blobStore = azureBlobStore;
8078
this.chunkSize = repositorySettings.settings().getAsBytesSize(AzureStorageService.Fields.CHUNK_SIZE,
8179
componentSettings.getAsBytesSize(AzureStorageService.Fields.CHUNK_SIZE, new ByteSizeValue(64, ByteSizeUnit.MB)));
8280

@@ -133,5 +131,37 @@ protected ByteSizeValue chunkSize() {
133131
return chunkSize;
134132
}
135133

134+
@Override
135+
public void initializeSnapshot(SnapshotId snapshotId, ImmutableList<String> indices, MetaData metaData) {
136+
try {
137+
if (!blobStore.client().doesContainerExist(blobStore.container())) {
138+
logger.debug("container [{}] does not exist. Creating...", blobStore.container());
139+
blobStore.client().createContainer(blobStore.container());
140+
}
141+
super.initializeSnapshot(snapshotId, indices, metaData);
142+
} catch (StorageException e) {
143+
logger.warn("can not initialize container [{}]: [{}]", blobStore.container(), e.getMessage());
144+
throw new SnapshotCreationException(snapshotId, e);
145+
} catch (URISyntaxException e) {
146+
logger.warn("can not initialize container [{}]: [{}]", blobStore.container(), e.getMessage());
147+
throw new SnapshotCreationException(snapshotId, e);
148+
}
149+
}
136150

151+
@Override
152+
public String startVerification() {
153+
try {
154+
if (!blobStore.client().doesContainerExist(blobStore.container())) {
155+
logger.debug("container [{}] does not exist. Creating...", blobStore.container());
156+
blobStore.client().createContainer(blobStore.container());
157+
}
158+
return super.startVerification();
159+
} catch (StorageException e) {
160+
logger.warn("can not initialize container [{}]: [{}]", blobStore.container(), e.getMessage());
161+
throw new RepositoryVerificationException(repositoryName, "can not initialize container " + blobStore.container(), e);
162+
} catch (URISyntaxException e) {
163+
logger.warn("can not initialize container [{}]: [{}]", blobStore.container(), e.getMessage());
164+
throw new RepositoryVerificationException(repositoryName, "can not initialize container " + blobStore.container(), e);
165+
}
166+
}
137167
}

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

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,12 @@
3131
import org.elasticsearch.cloud.azure.AzureStorageService;
3232
import org.elasticsearch.cluster.ClusterState;
3333
import org.elasticsearch.common.Strings;
34+
import org.elasticsearch.common.base.Predicate;
3435
import org.elasticsearch.common.settings.ImmutableSettings;
3536
import org.elasticsearch.common.settings.Settings;
3637
import org.elasticsearch.repositories.RepositoryException;
3738
import org.elasticsearch.repositories.RepositoryMissingException;
39+
import org.elasticsearch.repositories.RepositoryVerificationException;
3840
import org.elasticsearch.snapshots.SnapshotMissingException;
3941
import org.elasticsearch.snapshots.SnapshotState;
4042
import org.elasticsearch.test.ElasticsearchIntegrationTest;
@@ -44,6 +46,7 @@
4446
import org.junit.Test;
4547

4648
import java.net.URISyntaxException;
49+
import java.util.concurrent.TimeUnit;
4750

4851
import static org.hamcrest.Matchers.*;
4952

@@ -388,6 +391,47 @@ public void testNonExistingRepo_23() {
388391
}
389392
}
390393

394+
/**
395+
* When a user remove a container you can not immediately create it again.
396+
*/
397+
@Test
398+
public void testRemoveAndCreateContainer() throws URISyntaxException, StorageException, InterruptedException {
399+
final String container = getContainerName();
400+
final AzureStorageService storageService = internalCluster().getInstance(AzureStorageService.class);
401+
402+
// It could happen that we run this test really close to a previous one
403+
// so we might need some time to be able to create the container
404+
assertThat(awaitBusy(new Predicate<Object>() {
405+
public boolean apply(Object obj) {
406+
try {
407+
storageService.createContainer(container);
408+
logger.debug(" -> container created...");
409+
return true;
410+
} catch (URISyntaxException e) {
411+
// Incorrect URL. This should never happen.
412+
return false;
413+
} catch (StorageException e) {
414+
// It could happen. Let's wait for a while.
415+
logger.debug(" -> container is being removed. Let's wait a bit...");
416+
return false;
417+
}
418+
}
419+
}, 30, TimeUnit.SECONDS), equalTo(true));
420+
storageService.removeContainer(container);
421+
422+
ClusterAdminClient client = client().admin().cluster();
423+
logger.info("--> creating azure repository while container is being removed");
424+
try {
425+
client.preparePutRepository("test-repo").setType("azure")
426+
.setSettings(ImmutableSettings.settingsBuilder()
427+
.put(AzureStorageService.Fields.CONTAINER, container)
428+
).get();
429+
fail("we should get a RepositoryVerificationException");
430+
} catch (RepositoryVerificationException e) {
431+
// Fine we expect that
432+
}
433+
}
434+
391435
/**
392436
* Deletes repositories, supports wildcard notation.
393437
*/

0 commit comments

Comments
 (0)