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

Add support for secondary azure storage account #13779

Merged
merged 1 commit into from Nov 18, 2015

Conversation

Projects
None yet
5 participants
@dadoonet
Copy link
Member

dadoonet commented Sep 24, 2015

In #13228, @craigwi proposed a PR to support multiple storage accounts and secondary endpoints (which are readonly).

As per discussion on the thread I started to revisit the PR.

This PR rebased @craigwi work on master and modify the way you define secondary storage account.

In elasticsearch.yml:

cloud:
    azure:
        storage:
            my_account1:
                account: your_azure_storage_account1
                key: your_azure_storage_key1
                default: true
            my_account2:
                account: your_azure_storage_account2
                key: your_azure_storage_key2

When creating a repository, you can choose which azure account you want to use for it:

# This one will use the default account (my_account1)
curl -XPUT localhost:9200/_snapshot/my_backup1?pretty -d '{
  "type": "azure"
}'

# This one will use the second account (it will make it readonly)
curl -XPUT localhost:9200/_snapshot/my_backup2?pretty -d '{
  "type": "azure",
  "settings": { 
    "account" : "my_account2",
    "location_mode": "secondary_only"
  }
}'

@craigwi I ran some manual tests and it seems to work as expected but I'd love if you could confirm that.
Do you think you can build my branch and run a test on your side?

I don't think it's totally ready for a review though (I want at least to squash/reword some of commits).

@craigwi

This comment has been minimized.

Copy link
Contributor

craigwi commented Sep 25, 2015

Thanks @dadoonet. I'll review and get back to you shortly.

@dadoonet dadoonet force-pushed the dadoonet:pr/azure-multi-account branch Oct 12, 2015

@dadoonet

This comment has been minimized.

Copy link
Member Author

dadoonet commented Oct 12, 2015

@craigwi FYI I added a new commit and rebased on master.

@dadoonet dadoonet force-pushed the dadoonet:pr/azure-multi-account branch to 0057dee Oct 27, 2015

@dadoonet

This comment has been minimized.

Copy link
Member Author

dadoonet commented Oct 27, 2015

@imotov Do you think you could review this PR?

@craigwi

This comment has been minimized.

Copy link
Contributor

craigwi commented Oct 27, 2015

Thanks @dadoonet; as mentioned in a separate note, I have reviewed this and we are good to go.

try {
if (account != null) {
logger.trace("creating new Azure storage client using account [{}], key [{}], blob [{}]", account, key, blob);
String blob = "https://" + azureStorageSettings.getAccount() + ".blob.core.windows.net/";

This comment has been minimized.

Copy link
@imotov

imotov Nov 10, 2015

Member

Since the account settings are supplied by user, I would feels better if we used URI to build this string. This way we will have at least some basic validation of the things that go into this URL.

This comment has been minimized.

Copy link
@dadoonet

dadoonet Nov 18, 2015

Author Member

Well looking at the code again it sounds like we don't really use that blob string here. We just use it for logger.trace just below...

So I think I should remove that :D

@imotov

This comment has been minimized.

Copy link
Member

imotov commented Nov 10, 2015

Left one minor comment. The snapshot/restore side looks good to me.

@dadoonet dadoonet removed the v2.1.0 label Nov 18, 2015

Add support for secondary azure storage account
Follow up for #13228.

This commit adds support for a secondary storage account:

```yml
cloud:
    azure:
        storage:
            my_account1:
                account: your_azure_storage_account1
                key: your_azure_storage_key1
                default: true
            my_account2:
                account: your_azure_storage_account2
                key: your_azure_storage_key2
```

When creating a repository, you can choose which azure account you want to use for it:

```sh
curl -XPUT localhost:9200/_snapshot/my_backup1?pretty -d '{
  "type": "azure"
}'

curl -XPUT localhost:9200/_snapshot/my_backup2?pretty -d '{
  "type": "azure",
  "settings": {
    "account" : "my_account2",
    "location_mode": "secondary_only"
  }
}'
```

`location_mode` supports `primary_only` or `secondary_only`. Defaults to `primary_only`. Note that if you set it
to `secondary_only`, it will force `read_only` to true.

@dadoonet dadoonet force-pushed the dadoonet:pr/azure-multi-account branch from 4a9546b to 79a4d9c Nov 18, 2015

@dadoonet dadoonet merged commit 79a4d9c into elastic:master Nov 18, 2015

1 check passed

CLA Commit author has signed the CLA
Details

@dadoonet dadoonet deleted the dadoonet:pr/azure-multi-account branch Nov 18, 2015

@dadoonet

This comment has been minimized.

Copy link
Member Author

dadoonet commented Nov 18, 2015

Merged also in 2.x branch with 04b51d4

dadoonet added a commit to dadoonet/elasticsearch that referenced this pull request Nov 19, 2015

Filter cloud azure credentials
Bug introduced in elastic#13779: we don't filter anymore credentials because we were filtering `cloud.azure.storage.account` and `cloud.azure.storage.key` but now credentials are like `cloud.azure.storage.XXX.account` and `cloud.azure.storage.XXX.key` where `XXX` can be a storage setting id.

Closes elastic#14843.

dadoonet added a commit that referenced this pull request Nov 19, 2015

Filter cloud azure credentials
Bug introduced in #13779: we don't filter anymore credentials because we were filtering `cloud.azure.storage.account` and `cloud.azure.storage.key` but now credentials are like `cloud.azure.storage.XXX.account` and `cloud.azure.storage.XXX.key` where `XXX` can be a storage setting id.

Closes #14843.

(cherry picked from commit 149279f)
@dadoonet

This comment has been minimized.

This does not work! If you don't define any secondary storage (which happens 99% of the time), this will fail.

Will open an issue.

dadoonet added a commit to dadoonet/elasticsearch that referenced this pull request Jan 14, 2016

Fix Azure repository with only one primary account
Using a single azure account is now rejected.
This commit fixes this issue and adds a test for it.

This regression was introduced with elastic#13779. Hopefully no elasticsearch version has been released since then.

Needs to be merged in 2.2, 2.x and master branches.

dadoonet added a commit that referenced this pull request Jan 14, 2016

Fix Azure repository with only one primary account
Using a single azure account is now rejected.
This commit fixes this issue and adds a test for it.

This regression was introduced with #13779. Hopefully no elasticsearch version has been released since then.

Backport of #15982 in 2.x branch.

(cherry picked from commit ed45ad6)

dadoonet added a commit that referenced this pull request Jan 14, 2016

Fix Azure repository with only one primary account
Using a single azure account is now rejected.
This commit fixes this issue and adds a test for it.

This regression was introduced with #13779. Hopefully no elasticsearch version has been released since then.

Backport of #15982 in 2.2 branch.

(cherry picked from commit ed45ad6)
(cherry picked from commit 2fdcd35)

dadoonet added a commit to dadoonet/elasticsearch that referenced this pull request Jan 19, 2016

Add more tests for Azure Repository client selection
One test we forgot in elastic#14843 and elastic#13779 is the default client selection.

Most of the time, users won't define explicitly which client they want to use because they are providing only one connection to Azure storage:

```yml
cloud:
    azure:
        storage:
            my_account:
                account: your_azure_storage_account
                key: your_azure_storage_key
```

Then using the default client like this:

```sh
# This one will use the default account (my_account1)
curl -XPUT localhost:9200/_snapshot/my_backup1?pretty -d '{
  "type": "azure"
}'
```

This commit adds tests to check that the right client is still selected when no client is explicitly set when creating the snapshot.

dadoonet added a commit that referenced this pull request Jan 19, 2016

Add more tests for Azure Repository client selection
One test we forgot in #14843 and #13779 is the default client selection.

Most of the time, users won't define explicitly which client they want to use because they are providing only one connection to Azure storage:

```yml
cloud:
    azure:
        storage:
            my_account:
                account: your_azure_storage_account
                key: your_azure_storage_key
```

Then using the default client like this:

```sh
# This one will use the default account (my_account1)
curl -XPUT localhost:9200/_snapshot/my_backup1?pretty -d '{
  "type": "azure"
}'
```

This commit adds tests to check that the right client is still selected when no client is explicitly set when creating the snapshot.

Backport of #16088 in 2.2 branch

dadoonet added a commit that referenced this pull request Jan 19, 2016

Add more tests for Azure Repository client selection
One test we forgot in #14843 and #13779 is the default client selection.

Most of the time, users won't define explicitly which client they want to use because they are providing only one connection to Azure storage:

```yml
cloud:
    azure:
        storage:
            my_account:
                account: your_azure_storage_account
                key: your_azure_storage_key
```

Then using the default client like this:

```sh
# This one will use the default account (my_account1)
curl -XPUT localhost:9200/_snapshot/my_backup1?pretty -d '{
  "type": "azure"
}'
```

This commit adds tests to check that the right client is still selected when no client is explicitly set when creating the snapshot.

Backport of #16088 in 2.x branch

dadoonet added a commit to dadoonet/elasticsearch that referenced this pull request Feb 24, 2016

Fix Unknown [repository] type [azure] error with 2.2.0
This regression has been introduced in 2.2.0 by elastic#13779 where we removed support for `cloud.azure.storage.account` and replaced by `cloud.azure.storage.my_account.account`.
But Azure plugin tries to detect when it starts if all needed settings to use an `azure` repository have been set. If not the case, the plugin does not expose `azure` as an available repository.

Sadly, this check has been badly updated in 2.2.0 so it can never find the expected settings to start correctly.

This gives the following effect:

```yaml
cloud:
    azure:
        storage:
            my_account:
                account: your_azure_storage_account
                key: your_azure_storage_key
```

When you try to execute the following API you get Unknown [repository] type [azure]:

```sh
[msimos@msi-gs60 elasticsearch-2.2.0]$ curl -XPUT http://localhost:9200/_snapshot/mybackup?pretty -d '{
"type": "azure"
}'
```

```js
{
  "error" : {
    "root_cause" : [ {
      "type" : "repository_exception",
      "reason" : "[mybackup] failed to create repository"
    } ],
    "type" : "repository_exception",
    "reason" : "[mybackup] failed to create repository",
    "caused_by" : {
      "type" : "illegal_argument_exception",
      "reason" : "Unknown [repository] type [azure]"
    }
  },
  "status" : 500
}
```

```sh
[msimos@msi-gs60 elasticsearch-2.2.0]$ curl -XGE http://localhost:9200/_cat/plugins?v
name   component   version type url
node01 cloud-azure 2.2.0   j
node01 license     2.2.0   j
node01 shield      2.2.0   j
```
In the elasticsearch log file you see this error:

```
[2016-02-19 10:54:47,357][DEBUG][cloud.azure              ] [node01] starting azure services
[2016-02-19 10:54:47,357][DEBUG][cloud.azure              ] [node01] azure repository is not set using [repositories.azure.account] and [cloud.azure.storage.key] properties
```

Closes elastic#16734

dadoonet added a commit that referenced this pull request Feb 25, 2016

Fix Unknown [repository] type [azure] error with 2.2.0
This regression has been introduced in 2.2.0 by #13779 where we removed support for `cloud.azure.storage.account` and replaced by `cloud.azure.storage.my_account.account`.
But Azure plugin tries to detect when it starts if all needed settings to use an `azure` repository have been set. If not the case, the plugin does not expose `azure` as an available repository.

Sadly, this check has been badly updated in 2.2.0 so it can never find the expected settings to start correctly.

This gives the following effect:

```yaml
cloud:
    azure:
        storage:
            my_account:
                account: your_azure_storage_account
                key: your_azure_storage_key
```

When you try to execute the following API you get Unknown [repository] type [azure]:

```sh
[msimos@msi-gs60 elasticsearch-2.2.0]$ curl -XPUT http://localhost:9200/_snapshot/mybackup?pretty -d '{
"type": "azure"
}'
```

```js
{
  "error" : {
    "root_cause" : [ {
      "type" : "repository_exception",
      "reason" : "[mybackup] failed to create repository"
    } ],
    "type" : "repository_exception",
    "reason" : "[mybackup] failed to create repository",
    "caused_by" : {
      "type" : "illegal_argument_exception",
      "reason" : "Unknown [repository] type [azure]"
    }
  },
  "status" : 500
}
```

```sh
[msimos@msi-gs60 elasticsearch-2.2.0]$ curl -XGE http://localhost:9200/_cat/plugins?v
name   component   version type url
node01 cloud-azure 2.2.0   j
node01 license     2.2.0   j
node01 shield      2.2.0   j
```
In the elasticsearch log file you see this error:

```
[2016-02-19 10:54:47,357][DEBUG][cloud.azure              ] [node01] starting azure services
[2016-02-19 10:54:47,357][DEBUG][cloud.azure              ] [node01] azure repository is not set using [repositories.azure.account] and [cloud.azure.storage.key] properties
```

Closes #16734

(cherry picked from commit 7ffd6aa)
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.