-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
pkg/cloud/azure: Support specifying Azure environments in storage URLs #80511
pkg/cloud/azure: Support specifying Azure environments in storage URLs #80511
Conversation
Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR. Before a member of our team reviews your PR, I have some potential action items for you:
🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan. |
Thank you for updating your pull request. Before a member of our team reviews your PR, I have some potential action items for you:
🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan. |
ef46a67
to
918c7a9
Compare
Thank you for updating your pull request. My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan. |
The Azure Storage cloud provider learned a new parameter, AZURE_ENVIRONMENT, which specifies which azure environment the storage account in question belongs to. This allows cockroach to backup and restore data to Azure Storage Accounts outside the main Azure Public Cloud. For backwards compatibility, this defaults to "AzurePublicCloud" if AZURE_ENVIRONMENT is not specified. Fixes cockroachdb#47163 Release note (general): When using Azure Cloud Storage for data operations, cockroach now calculates the Storage Account URL from the provided AZURE_ENVIRONMENT query parameter. This defaults to AzurePublicCloud if not specified to maintain backwards compatibility.
918c7a9
to
4b97370
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nlowe-sx thank you for this contribution and the thorough testing, this looks good to me! I'll merge it for you.
bors r=adityamaru |
Build succeeded: |
@adityamaru Thanks for the review! Is it too late in the 22.1 release process to get this included? Also, what's the chance of getting this back-ported to 21.2? We'd love to start making use of this change but won't be upgrading to 22.1 for at least 6 months due to our upgrade policy, but we are planning on upgrading to 21.2 in a few weeks. |
We are discussing this internally as it is quite late in our release process to include this in our 22.1.0 GA. We might consider backporting it to a 22.1 dot release instead.
We're discussing this as well 😋 Given that the change is pretty well understood, I think it's possible. I'll open a backport and will update you with our decision early next week! |
blathers backport 21.2 |
blathers backport 22.1 |
backports sound fine to me as along as the release note on the backport is pretty strongly worded about avoiding usage in the mixed version / upgrade state (i.e. since any non-upgraded nodes will still send requests to the default URI, i.e. public cloud). |
@nlowe-sx just to close the loop here, we did backport the changes to 21.2 and the future 22.1.1. You should be able to pick this up in 21.2.11 which should be out in a couple of weeks. |
The Azure Storage cloud provider learned a new parameter, AZURE_ENVIRONMENT,
which specifies which azure environment the storage account in question
belongs to. This allows cockroach to backup and restore data to Azure
Storage Accounts outside the main Azure Public Cloud. For backwards
compatibility, this defaults to "AzurePublicCloud" if AZURE_ENVIRONMENT
is not specified.
Fixes #47163
Verification Evidence
I spun up a single node cluster:
I was then able to view the contents of a backup hosted in an azure
government storage account:
Omitting the
AZURE_ENVIRONMENT
parameter, we can see cockroachdefaults to the public cloud where my storage account does not exist:
Tests
Two new tests are added to verify that the storage account URL is correctly
built from the provided Azure Environment name, and that the Environment
defaults to the Public Cloud if unspecified for backwards compatibility. I
verified the existing tests pass against a government storage account after
specifying
AZURE_ENVIRONMENT
asAzureUSGovernmentCloud
in the backup URLquery parameters: