Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove auto creation of container for azure repository #22858

Merged

Conversation

Projects
None yet
3 participants
@dadoonet
Copy link
Member

commented Jan 30, 2017

Follow up of #22857 where we deprecate automatic creation of azure containers.

BTW I found that the AzureSnapshotRestoreServiceIntegTests does not bring any value because it runs basically a Snapshot/Restore operation on local files which we already test in core.

So instead of trying to fix it to make it pass with this PR, I simply removed it.

Remove auto creation of container for azure repository
Follow up of #22857 where we deprecate automatic creation of azure containers.

BTW I found that the `AzureSnapshotRestoreServiceIntegTests` does not bring any value because it runs basically a Snapshot/Restore operation on local files which we already test in core.

So instead of trying to fix it to make it pass with this PR, I simply removed it.

@dadoonet dadoonet merged commit 1898dc2 into elastic:master Jan 31, 2017

2 checks passed

CLA Commit author is a member of Elasticsearch
Details
elasticsearch-ci Build finished.
Details

@dadoonet dadoonet deleted the dadoonet:pr/remove-azure-container-auto-creation branch Jan 31, 2017

dadoonet added a commit to dadoonet/elasticsearch that referenced this pull request Sep 25, 2017

Azure snapshots can not be restored anymore
While working on elastic#26751 and doing some manual integration testing I found that this elastic#22858 removed an important line of our code:

`AzureRepository` overrides default `initializeSnapshot` method which creates metadata files and do other stuff.

But with PR elastic#22858, I wrote:

```java
    @override
    public void initializeSnapshot(SnapshotId snapshotId, List<IndexId> indices, MetaData clusterMetadata) {
        if (blobStore.doesContainerExist(blobStore.container()) == false) {
            throw new IllegalArgumentException("The bucket [" + blobStore.container() + "] does not exist. Please create it before " +
                " creating an azure snapshot repository backed by it.");
        }
    }
```

instead of

```java
    @override
    public void initializeSnapshot(SnapshotId snapshotId, List<IndexId> indices, MetaData clusterMetadata) {
        if (blobStore.doesContainerExist(blobStore.container()) == false) {
            throw new IllegalArgumentException("The bucket [" + blobStore.container() + "] does not exist. Please create it before " +
                " creating an azure snapshot repository backed by it.");
        }
        super.initializeSnapshot(snapshotId, indices, clusterMetadata);
    }
```

As we never call `super.initializeSnapshot(...)` files are not created and we can't restore what we saved.

Closes elastic#26777.
logger.debug("container [{}] does not exist. Creating...", blobStore.container());
blobStore.createContainer(blobStore.container());
}
super.initializeSnapshot(snapshotId, indices, clusterMetadata);

This comment has been minimized.

Copy link
@dadoonet

dadoonet Sep 25, 2017

Author Member

This line has been removed by mistake with this PR. See #26777

jasontedor added a commit that referenced this pull request Sep 25, 2017

Azure snapshots can not be restored anymore (#26778)
While working on #26751 and doing some manual integration testing I found that this #22858 removed an important line of our code:

`AzureRepository` overrides default `initializeSnapshot` method which creates metadata files and do other stuff.

But with PR #22858, I wrote:

```java
    @override
    public void initializeSnapshot(SnapshotId snapshotId, List<IndexId> indices, MetaData clusterMetadata) {
        if (blobStore.doesContainerExist(blobStore.container()) == false) {
            throw new IllegalArgumentException("The bucket [" + blobStore.container() + "] does not exist. Please create it before " +
                " creating an azure snapshot repository backed by it.");
        }
    }
```

instead of

```java
    @override
    public void initializeSnapshot(SnapshotId snapshotId, List<IndexId> indices, MetaData clusterMetadata) {
        if (blobStore.doesContainerExist(blobStore.container()) == false) {
            throw new IllegalArgumentException("The bucket [" + blobStore.container() + "] does not exist. Please create it before " +
                " creating an azure snapshot repository backed by it.");
        }
        super.initializeSnapshot(snapshotId, indices, clusterMetadata);
    }
```

As we never call `super.initializeSnapshot(...)` files are not created and we can't restore what we saved.

Closes #26777.

jasontedor added a commit that referenced this pull request Sep 25, 2017

Azure snapshots can not be restored anymore (#26778)
While working on #26751 and doing some manual integration testing I found that this #22858 removed an important line of our code:

`AzureRepository` overrides default `initializeSnapshot` method which creates metadata files and do other stuff.

But with PR #22858, I wrote:

```java
    @override
    public void initializeSnapshot(SnapshotId snapshotId, List<IndexId> indices, MetaData clusterMetadata) {
        if (blobStore.doesContainerExist(blobStore.container()) == false) {
            throw new IllegalArgumentException("The bucket [" + blobStore.container() + "] does not exist. Please create it before " +
                " creating an azure snapshot repository backed by it.");
        }
    }
```

instead of

```java
    @override
    public void initializeSnapshot(SnapshotId snapshotId, List<IndexId> indices, MetaData clusterMetadata) {
        if (blobStore.doesContainerExist(blobStore.container()) == false) {
            throw new IllegalArgumentException("The bucket [" + blobStore.container() + "] does not exist. Please create it before " +
                " creating an azure snapshot repository backed by it.");
        }
        super.initializeSnapshot(snapshotId, indices, clusterMetadata);
    }
```

As we never call `super.initializeSnapshot(...)` files are not created and we can't restore what we saved.

Closes #26777.

jasontedor added a commit that referenced this pull request Sep 25, 2017

Azure snapshots can not be restored anymore (#26778)
While working on #26751 and doing some manual integration testing I found that this #22858 removed an important line of our code:

`AzureRepository` overrides default `initializeSnapshot` method which creates metadata files and do other stuff.

But with PR #22858, I wrote:

```java
    @override
    public void initializeSnapshot(SnapshotId snapshotId, List<IndexId> indices, MetaData clusterMetadata) {
        if (blobStore.doesContainerExist(blobStore.container()) == false) {
            throw new IllegalArgumentException("The bucket [" + blobStore.container() + "] does not exist. Please create it before " +
                " creating an azure snapshot repository backed by it.");
        }
    }
```

instead of

```java
    @override
    public void initializeSnapshot(SnapshotId snapshotId, List<IndexId> indices, MetaData clusterMetadata) {
        if (blobStore.doesContainerExist(blobStore.container()) == false) {
            throw new IllegalArgumentException("The bucket [" + blobStore.container() + "] does not exist. Please create it before " +
                " creating an azure snapshot repository backed by it.");
        }
        super.initializeSnapshot(snapshotId, indices, clusterMetadata);
    }
```

As we never call `super.initializeSnapshot(...)` files are not created and we can't restore what we saved.

Closes #26777.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.