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

[Service Bus]"Preparer failure when creating resource ServiceBusNamespacePreparer, Resource group could not be found" on UsGov and China cloud #20517

Closed
v-xuto opened this issue Sep 2, 2021 · 35 comments · Fixed by #29202
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. Messaging Messaging crew Service Bus test-sovereign-cloud
Milestone

Comments

@v-xuto
Copy link
Member

v-xuto commented Sep 2, 2021

We are running live Tests against other clouds like US Gov and Azure China Cloud. The goal is to check whether new azure sdk package work with other clouds or not.

Error Description:
When running the test tests\test_queues.py::ServiceBusQueueTests::test_github_issue_6178 on UsGov and China cloud, it runs failed and the error message is shown as following, for more details please check here:
image

Error Track:
The code that failed is here
image

code link2:
image

image

Note: We guessed that the environment variable did not set the value of RESOURCE_REGION, so the default value ‘westus’ was used, so the resource group was not created successfully in UsGov and China Cloud.

Expected Behavior:
The resource group was successfully created and it can be found.

@benbp , @jameszliao-msft , @lmazuel , @lilyjma , @ramya-rao-a and @annatisch for notification.

@v-xuto v-xuto changed the title [Service Bus]"Preparer failure when creating resource ServiceBusNamespacePreparer, Resource group '***' could not be found" on UsGov and China cloud [Service Bus]"Preparer failure when creating resource ServiceBusNamespacePreparer, Resource group could not be found" on UsGov and China cloud Sep 2, 2021
@v-xuto v-xuto added Client This issue points to a problem in the data-plane of the library. needs-team-triage Workflow: This issue needs the team to triage. Service Bus test-sovereign-cloud labels Sep 2, 2021
@benbp
Copy link
Member

benbp commented Sep 2, 2021

If you change RESOURCE_REGION to SERVICEBUS_LOCATION it should be able to look up the value correctly based on the context set by the resource deployment scripts.

@v-xuto
Copy link
Member Author

v-xuto commented Sep 10, 2021

@benbp

  1. First, because get_region_override is a method in tools devtools_testutils, so we update location=get_region_override('westus') to location=os.environ.get("SERVICEBUS_LOCATION", None) or 'westus' in class ServiceBusNamespacePreparer(AzureMgmtPreparer). But it still reports the same error, its location is still westus. We suspect that this parameter SERVICEBUS_LOCATION has no value or is 'westus' ?
  2. Then, we update location=get_region_override('westus') to location='usgovarizona', it reports the following error, for more details in here:
(test_queues.ServiceBusQueueTests): (LocationNotAvailableForResourceType) The provided location '***' is not available for resource type 'Microsoft.ServiceBus/namespaces'. List of available regions for the resource type is 'australiaeast,australiasoutheast,centralus,eastus,eastus2,westus2,westus,northcentralus,southcentralus,westcentralus,eastasia,southeastasia,brazilsouth,japaneast,japanwest,northeurope,westeurope,centralindia,southindia,westindia,canadacentral,canadaeast,ukwest,uksouth,koreacentral,koreasouth,francecentral,southafricanorth,uaenorth,australiacentral,switzerlandnorth,germanywestcentral,norwayeast,jioindiawest,westus3,centraluseuap,eastus2euap'.

By reporting the error message, we suspect that it is still under the public cloud. This is confusing, where is it going to the corresponding cloud environment.

Do you have any ideas?

@benbp
Copy link
Member

benbp commented Sep 10, 2021

@v-xuto sorry this is my mistake! Looks like it's called SERVICE_BUS_LOCATION. You can see the key names of the env outputs that get set in the Deploy test resources logs:

https://dev.azure.com/azure-sdk/internal/_build/results?buildId=1080695&view=logs&j=eb4e8fab-410d-5462-8da4-6074733a2525&t=5877313e-8ad1-5e16-7cf7-7ae7932c113c&l=3267

@yunhaoling yunhaoling added Messaging Messaging crew and removed needs-team-triage Workflow: This issue needs the team to triage. labels Sep 13, 2021
@v-xuto
Copy link
Member Author

v-xuto commented Sep 16, 2021

@benbp We updated SERVICEBUS_LOCATION to SERVICE_BUS_LOCATION, there is still the same error, its location is still westus, for more details please check here. Besides, we update location=get_region_override('westus') to location='usgovarizona', by reporting the error message, we suspect that it is still under the public cloud.

@benbp
Copy link
Member

benbp commented Sep 16, 2021

@v-xuto I didn't see that it was in the dev tools location. You might try instead setting RESOURCE_REGION as an output value in the test-resources.json corresponding to [resourceGroup().location].

Looking at your branch, you also need to update the azure environment values to reference the dynamic cloud subscription config variables instead of the hardcoded public cloud variable group. That's why it's looking in public:

AZURE_SUBSCRIPTION_ID: $(azure-subscription-id)
AZURE_TENANT_ID: $(aad-azure-sdk-test-tenant-id)
AZURE_CLIENT_ID: $(aad-azure-sdk-test-client-id)
AZURE_CLIENT_SECRET: $(aad-azure-sdk-test-client-secret)

https://dev.azure.com/azure-sdk/internal/_build/results?buildId=1093075&view=logs&j=eb4e8fab-410d-5462-8da4-6074733a2525&t=5877313e-8ad1-5e16-7cf7-7ae7932c113c&l=3300

        AZURE_SUBSCRIPTION_ID: $(SERVICEBUS_SUBSCRIPTION_ID)
        AZURE_TENANT_ID: $(SERVICEBUS_TENANT_ID)
        AZURE_CLIENT_ID: $(SERVICEBUS_CLIENT_ID)
        AZURE_CLIENT_SECRET: $(SERVICEBUS_CLIENT_SECRET)

@v-xuto
Copy link
Member Author

v-xuto commented Sep 24, 2021

@benbp - I have updated the code by your comments. But we encountered the following error, for more details please check here:
Usgov:
azure_devtools.scenario_tests.preparers:preparers.py:82 Preparer failure when creating resource ServiceBusNamespacePreparer for test test_github_issue_6178 (test_queues.ServiceBusQueueTests): , AdalError: Get Token request returned http error: 400 and server response: {"error":"invalid_request","error_description":"AADSTS900382: Confidential Client is not supported in Cross Cloud request.}

China:
azure_devtools.scenario_tests.preparers:preparers.py:82 Preparer failure when creating resource ***erviceBusNamespacePreparer for test test_github_issue_6178 (test_queues.***erviceBusQueueTests): , AdalError: Get Token request returned http error: 400 and server response: {"error":"invalid_request","error_description":"AAD***T***90002: Tenant '***' not found. Check to make sure you have the correct tenant ID and are signing into the correct cloud. Check with your subscription administrator, this may happen if there are no active subscriptions for the tenant.}

Do you have any ideas?

@benbp
Copy link
Member

benbp commented Sep 28, 2021

@v-xuto it seems like the tenant id is still not getting set for the sovereign cloud properly, which is generally the issue for errors like not supported in Cross Cloud request. @scbedd I tried to dig a little through the devtools to see whether a custom tenant id isn't getting passed through properly, but I couldn't find anything that indicated a problem. Perhaps you have an idea?

@scbedd
Copy link
Member

scbedd commented Sep 28, 2021

@benbp If all the surface level stuff appears legit, it's going to be something esoteric. Is this the only python package having issues cross-cloud?

Following up offline to get a bit more context.

@v-xuto
Copy link
Member Author

v-xuto commented Oct 8, 2021

@scbedd Yeah, currently I have tested the formrecognizer, eventhubs, textanalytics and appconfig, none of them have this problem.
@benbp Is there anything to add?

@scbedd
Copy link
Member

scbedd commented Oct 11, 2021

Ok if all the others work, it's either A) configuration issue with the servicebus tests specifically (I'm leaning towards this, servicebus hasn't gotten a TON of attention) or B) a service specific issue that we're identifying.

@v-xuto
Copy link
Member Author

v-xuto commented Oct 12, 2021

@benbp What do you think about these comments? Do you think it is a servicebus configuration problem? How do we solve the issue?

@benbp
Copy link
Member

benbp commented Oct 12, 2021

@yunhaoling can you look at this in addition to #20875? Or is there a better sdk service owner?

@yunhaoling yunhaoling assigned yunhaoling and unassigned chradek Oct 13, 2021
@yunhaoling
Copy link
Contributor

yunhaoling commented Oct 13, 2021

@benbp , I will first try to verify it locally before deploying it to CI, if it can't work locally, then might be an issue in the SDK

@v-xuto
Copy link
Member Author

v-xuto commented Nov 4, 2021

@yunhaoling Any progress?

@v-xuto
Copy link
Member Author

v-xuto commented Dec 16, 2021

@benbp, @yunhaoling By my investigation locally, the errors on usgov and china should be related to the scope https://management.azure.com in mgmt sdk azure-mgmt-resource and azure-mgmt-servicebus. Because I modified the scope (https://management.azure.com) value on usgov and china, the local result of the servicebus test was pass. When the cloud is usgov, its value should be https://management.usgovcloudapi.net; when the cloud is china, its value should be https://management.chinacloudapi.net. In servicebus tests code, it needs to create a resource group and service bus by itself, but the scope in its source code is hard-coded, it is https://management.usgovcloudapi.net. It is similar to the issue of eventhub. I currently have two ideas to solve this problem.

  1. File issues about azure-mgmt-resource and azure-mgmt-servicebus. Do you know in which repo I should file mgmt issues? I am not very clear.
    eg:
    azure/mgmt/servicebus/_service_bus_management_client.py
    image
    azure/mgmt/servicebus/_configuration.py
    image

  2. Can we consider changing the test framework and directly use the service created in this part of Deploy test resources for testing. Don't use mgmt sdk to create resources in the test, like key vault keys and app configuration tests.

Regarding these two methods, which one do you prefer? Please give me some advice.

@benbp
Copy link
Member

benbp commented Dec 17, 2021

@v-xuto I think we should change places in the test framework that are causing problems, and we should not be deploying via the test code, only the ARM template. I think this is mostly done across the python repository but there are still a few places that use the legacy deployment method. @yunhaoling do you know if/where this work is tracked, or am I wrong?

@v-xuto
Copy link
Member Author

v-xuto commented Dec 23, 2021

@yunhaoling Do you have any ideas?

@yunhaoling
Copy link
Contributor

yunhaoling commented Dec 23, 2021

hey @benbp @v-xuto , apologize for missing the threads here. I agree that we should update the code within the test framework (to be more specific, it's ServiceBusPreparer) to get tests work on UsGov and China Cloud. However, I think it might take extra time if we want to embrace the ARM template approach completely using PowerShellPreparer (or even not feasible).

Here's my findings:

  1. @benbp, as far as I know, we use ARM template + PowerShellPreparer to deploy resources for sample tests (Deploy test resources), but for live tests, each SDK does it differently, some using MgmtClient while some using ARM template. I think it depends on the SDKs and test scenarios.
  • for service bus live tests, we need different queues to test various queue/topic/subscription settings.
  1. @v-xuto thanks for you investigation, as you pointed out, I think it's because when we're creating the ServiceBusManagementClient in the preparer, we didn't consider other clouds having different base url leading us not to have the option to set the value.

I think there is one easy fix:

  1. In servicebus_preparer.py

within each create_resource method, we could read the base_url/credential_scopes information from the environment and pass it to the mgmt client like this:

    def create_resource(self, name, **kwargs):
        if self.is_live:
            base_url = os.envrion.get("BASE_URL","https://management.azure.com")
            credential_scopes = [os.envrion.get("CREDENTIAL_SCOPES","https://management.azure.com/.default")]
            self.client = self.create_mgmt_client(ServiceBusManagementClient, base_url=base_url, credential_scopes=credential_scopes)
            group = self._get_resource_group(**kwargs)
       ...
  1. in test.yml, we set the environment variables accordingly:
BASE_URL: https://management.usgovcloudapi.net
REDENTIAL_SCOPE: https://management.usgovcloudapi.net/.default
or
BASE_URL: https://management.chinacloudapi.net
CREDENTIAL_SCOPE: https://management.chinacloudapi.net/.default

However, I am unsure whether those envs could just be put under the CloudConfig section like this or has to be in other format, @benbp would you be able to help tell what is the right format?

        UsGov:
          SubscriptionConfiguration: $(sub-config-gov-test-resources)
          BASE_URL: https://management.usgovcloudapi.net
          REDENTIAL_SCOPE: https://management.usgovcloudapi.net/.default
        China:
          SubscriptionConfiguration: $(sub-config-cn-test-resources)
          Location: 'chinanorth2'
          BASE_URL: https://management.chinacloudapi.net
          CREDENTIAL_SCOPE: https://management.chinacloudapi.net/.default

@v-xuto , I think you could try step 1 with local envs to see if it works, if this approach works, then same change could be applied to your EH PR.

@v-xuto
Copy link
Member Author

v-xuto commented Dec 24, 2021

@yunhaoling Thanks for your help. I have tried step 1 with local envs and also add same code in Create Resource Group Management Client in file devtools_testutils/resource_testcase.py. And then, the service bus test result is pass locally.
image

Besides, can I directly modify the code in tools devtools_testutils?

@benbp Please help to set environment variables CREDENTIAL_SCOPES and BASE_URL in China and UsGov clouds, like the parameter Azure_Authority_Host.

@yunhaoling
Copy link
Contributor

yunhaoling commented Dec 27, 2021

@v-xuto I think instead of updating the resource_testcase.py, it's safer to just update the servicebus_preparer.py as we might break other SDKs due to how we construct the envs here.

Note that there're six create_resource in the service_preparer.py which need to be updated.

@v-xuto
Copy link
Member Author

v-xuto commented Feb 28, 2022

@rakshith91 Do you have any ideas instead of modifying the resource_testcase.py file?

@rakshith91
Copy link
Contributor

@v-xuto im currently working on getting rid of it completely and using the arm template. That will fix this issue. It should be done in a couple of weeks. Sorry for the late update.

@v-xuto
Copy link
Member Author

v-xuto commented Mar 25, 2022

@rakshith91 How is the current progress of this issue?

@rakshith91
Copy link
Contributor

@v-xuto I have a PR out - i'm hoping to get it merged by next week
#23566

@lmazuel lmazuel modified the milestones: [2022] March, [2022] May Apr 13, 2022
@v-xuto
Copy link
Member Author

v-xuto commented May 13, 2022

@rakshith91 How is your PR going so far? When will this PR be merged?
I'm looking forward to your PR being merged soon.

@rakshith91
Copy link
Contributor

@v-xuto I have a few more tests to fix before merging the PR. Meanwhile, feel free to test it on my branch. You'd be seeing some test failures, but no set up failures.

cc: @kashifkhan

@v-xuto
Copy link
Member Author

v-xuto commented May 16, 2022

@rakshith91 Thank you for your prompt reply. I'm looking forward to your PR being merged soon.

@lmazuel lmazuel modified the milestones: [2022] May, [2022] June May 16, 2022
@rakshith91 rakshith91 modified the milestones: [2022] June, [2022] July Jun 7, 2022
@v-xuto
Copy link
Member Author

v-xuto commented Aug 26, 2022

@rakshith91 How is your PR going so far to fix this issue?

@lmazuel lmazuel modified the milestones: 2022-07, 2022-11 Sep 6, 2022
azure-sdk pushed a commit to azure-sdk/azure-sdk-for-python that referenced this issue Sep 19, 2022
[Microsoft.StorageSync] New API version 2022-06-01 (Azure#20227)

* Adds base for updating Microsoft.StorageSync from version stable/2020-09-01 to version 2022-06-01

* Updates readme

* Updates API version in new specs and examples

* Swagger API Spec Update for adding Low Disk Mode to Storage Sync (Azure#19725)

* Added Low Disk Mode property

* Updated API Spec for introducing Low Disk Mode

* Updated Examples for API Spec for Low Disk Mode

* Updated Examples for Cloud Tiering Status (Azure#19885)

* [Microsoft.StorageSync] Add AFS file share metadata certificate public keys API to version 2022-06-01 (S360 compliance) (Azure#20517)

* Fix readme.md file so that tag information includes full API version 2022-06-01

* Add afs share metadata certificate public keys API

https://portal.azure-devex-tools.com/amekpis/completeness/detail?errorId=DBF8D800-EB0B-48E0-B665-BC91CB1A17B9

* Fix casing of afssharemetadatacertificatepublickeys

* Temporarily rename examples file to fix casing issue.

* Fix casing of new example file

Co-authored-by: ankushbindlish2 <[email protected]>
Co-authored-by: Juan Carlos Juarez <[email protected]>
@lmazuel lmazuel assigned kashifkhan and unassigned rakshith91 Oct 24, 2022
@lmazuel lmazuel modified the milestones: 2022-11, 2022-12 Oct 24, 2022
@v-xuto
Copy link
Member Author

v-xuto commented Mar 24, 2023

@swathipil What is the current progress on the fix of this issue?

@swathipil
Copy link
Member

Hi @v-xuto - I have a PR to fix this and all the tests are passing: #29202

However, CI is failing b/c of a dependency conflict. I have a PR out to fix that dependency conflict. Hoping we can merge all the PRs and close this issue by today. Thanks for your patience!

@ghost ghost closed this as completed in #29202 Mar 25, 2023
ghost pushed a commit that referenced this issue Mar 25, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jun 23, 2023
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. Messaging Messaging crew Service Bus test-sovereign-cloud
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants