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

Update keyvault to enable live testing in sovereign clouds for multiple services #21717

Merged
merged 3 commits into from
Dec 15, 2021
Merged

Update keyvault to enable live testing in sovereign clouds for multiple services #21717

merged 3 commits into from
Dec 15, 2021

Conversation

v-xuto
Copy link
Member

@v-xuto v-xuto commented Nov 11, 2021

These changes enable keyvault to run live tests against Public, UsGov and China.

  1. According to JS issue [Key Vault Keys] "Hardware key operation not allowed on standard vault" on UsGov and China cloud azure-sdk-for-js#18267, there is the same error here .
    So skip the following tests:
  • test_key_client.py::KeyClientTests::test_key_crud_operations,
  • test_keys_async.py::KeyVaultKeyTest::test_key_crud_operations,
  • test_samples_keys.py::TestExamplesKeyVault::test_example_key_crud_operations,
  • test_samples_keys_async.py::TestExamplesKeyVault::test_example_key_crud_operations
  1. According to JS issue [Key Vault Keys]Automatic key rotation is not supported on UsGov cloud and China cloud azure-sdk-for-js#18142, there is the same error here.
    So skip the following tests:
  • test_key_client.py::KeyClientTests::test_key_rotation,
  • test_key_client.py::KeyClientTests::test_key_rotation_policy,
  • test_keys_async.py::KeyVaultKeyTest::test_key_rotation,
  • test_keys_async.py::KeyVaultKeyTest::test_key_rotation_policy
  1. According to JS issue [Key Vault Keys] ResourceType "Microsoft.KeyVault/managedHSMs" deploys failed on UsGov and China cloud azure-sdk-for-js#18414, there is the same error here.
    So add code MatrixFilters:ArmTemplateParameters=^(?!.*enableHsm.*true) in UsGov and China cloudconfig in tests.yml.

Pipeline results:
https://dev.azure.com/azure-sdk/internal/_build/results?buildId=1194909&view=results

@benbp , @jameszliao-msft , @lmazuel , @AlexGhiondea , @chlowell for notification.

@ghost ghost added the KeyVault label Nov 11, 2021
@Azure Azure deleted a comment from azure-pipelines bot Nov 12, 2021
@v-xuto v-xuto marked this pull request as ready for review November 15, 2021 07:08
@v-xuto
Copy link
Member Author

v-xuto commented Nov 17, 2021

@benbp Could you please review this PR?

@benbp
Copy link
Member

benbp commented Nov 17, 2021

@v-xuto pipeline changes look good to me, but I defer to a language owner for reviewing the test code changes.

@benbp
Copy link
Member

benbp commented Dec 8, 2021

/azp run python - keyvault - tests-weekly

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@benbp
Copy link
Member

benbp commented Dec 8, 2021

@v-xuto assuming the tests pass is this ready to merge? It looks like there were some external dependency issues from yesterday's failure, so I'm re-running them.

@mikeharder
Copy link
Member

@benbp: I cancelled the run of "python - keyvault - tests-weekly". It was failing due to the DevOps cache task bug, and we're running low on agents due to provisioning issues.

@mccoyp
Copy link
Member

mccoyp commented Dec 9, 2021

@v-xuto I think you'll have to rebase this PR with the latest main to get the pipeline bug fix. @mikeharder or @benbp, please correct me if I'm wrong

@v-xuto v-xuto requested a review from scbedd as a code owner December 9, 2021 02:21
@v-xuto
Copy link
Member Author

v-xuto commented Dec 9, 2021

@mccoyp I have rebased this PR with the latest main. Then I rerun a new weekly pipeline, but it also failed due to the DevOps cache task bug. For more details please check here. About the ci pipeline error, it failed in the Analyze dependencies section.

@mccoyp
Copy link
Member

mccoyp commented Dec 10, 2021

@v-xuto Sorry about the failures! The Analyze dependencies error was just fixed earlier today with this PR, so another rebase (or copying the contents into this branch's shared_requirements.txt) should fix that issue.

As for the caching issue, that's something we're still working on. I made a temporary fix in the weekly tests pipeline, so re-running it should hopefully work. If it still fails, just let me know and we can provide an update when a more permanent fix is out.

@v-xuto
Copy link
Member Author

v-xuto commented Dec 10, 2021

@mccoyp Thanks for your help. Both of these issues have been resolved. I have rerun a new weekly pipeline, but it timed out in the Deploy test resources section in ubuntu1804_ManagedHSM_39 about public_azure_keyvault_keys and public_azure_keyvault_administration.
For more details please check here.

@benbp
Copy link
Member

benbp commented Dec 10, 2021

@v-xuto we could potentially be running to conflicts with other HSM jobs in the same region depending on when it's ran. There's a strict quota for HSM resources per region within the same subscription, so it's possible that's the issue. I'm re-running to see if it works again.

@v-xuto
Copy link
Member Author

v-xuto commented Dec 14, 2021

@benbp This quota for HSM resources issue still exists when I run it in the last three days. Do we hold it temporarily or do you have any ideas to solve this problem?

@benbp
Copy link
Member

benbp commented Dec 14, 2021

@mccoyp it seems the HSM tests on python in general are failing/timing out. I wonder if the issue might be that HSM tests are running for both keys and administration (https://github.com/Azure/azure-sdk-for-python/blob/main/sdk/keyvault/tests.yml#L20), meaning it will try to provision two HSM resources in the same region+subscription causing quota issues/delays?

@mccoyp
Copy link
Member

mccoyp commented Dec 14, 2021

@benbp True, I've been trying to figure out what's going on with HSM deployment for live pipelines. I don't think it's a quota issue; the service allowed five HSMs per region per subscription in preview, and there are usually no HSMs active in westus (each language team for KV uses a different region for pipeline testing). I was thinking that it could be an issue with test-resources-post.ps1, but after digging into it further, I noticed that HSM deployment stopped working last week without any code having been changed.

This build from 12/8 had some service-related failures, but HSMs got deployed correctly. Here is the test-resources.json file from the commit targeted in that build.

The next day, on 12/9, resource deployment failed in this build. The test-resources.json file from the build commit is identical to the one from the day before, and both commits have the same test-resources-post.ps1 file as well. This makes me think that it must be a service issue, but .NET pipelines are having no issues with HSM deployment despite using the same test-resources.json (with minor differences like HSM region).

I do think it could be a good idea to deploy only one HSM for pipeline testing, and it might resolve this issue if simultaneous deployment is the problem.

@benbp
Copy link
Member

benbp commented Dec 14, 2021

@mccoyp perhaps we should try changing the HSM region and seeing if that works. Maybe there's a current issue in the westus region.

Also if this is an active/known issue for public, I say we merge this PR to get the usgov/china testing going.

@mccoyp
Copy link
Member

mccoyp commented Dec 15, 2021

I created a PR here to unblock HSM deployment

@benbp
Copy link
Member

benbp commented Dec 15, 2021

Thanks @v-xuto!

@benbp benbp merged commit 1da8500 into Azure:main Dec 15, 2021
hildurhodd pushed a commit to hildurhodd/azure-sdk-for-python that referenced this pull request Jan 4, 2022
…le services (Azure#21717)

* Update keyvault to enable live testing in sovereign clouds for multiple services

* Add async test recordings

* Add test_key_crud_operations recordings
rakshith91 pushed a commit to rakshith91/azure-sdk-for-python that referenced this pull request Apr 10, 2022
…le services (Azure#21717)

* Update keyvault to enable live testing in sovereign clouds for multiple services

* Add async test recordings

* Add test_key_crud_operations recordings
azure-sdk pushed a commit to azure-sdk/azure-sdk-for-python that referenced this pull request Dec 6, 2022
[Hub Generated] Publish private branch 'user/pri/2022-12-01' (Azure#21717)

* add or modify files

* Fixed spellcheck validation err

* Fixed avocado validation err

* Fixed avocado validation err

* Fixed avocado validation err

* Fixed avocado validation err

* Fixed avocado validation err

* Fixed avocado validation err

* Fixed avocado validation err

* Fixed avocado validation err

* avacado fix

* avacado fix

* avacado fix

* avacado fix'

* avacado fix

* avacado fix

* revert custom-words.txt

* update custom-words.txt

* updated PreCheckResult

* removed SystemData

* removed SystemData

Co-authored-by: Aditya Ravishankar <ravishankara@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants