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 EventHub to enable live testing in sovereign clouds for multiple services #21715

Merged
25 commits merged into from
Mar 8, 2023
Merged

Update EventHub to enable live testing in sovereign clouds for multiple services #21715

25 commits merged into from
Mar 8, 2023

Conversation

v-xuto
Copy link
Member

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

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

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

@ghost ghost added the Event Hubs label Nov 11, 2021
@v-xuto v-xuto marked this pull request as ready for review November 11, 2021 09:06
@yunhaoling
Copy link
Contributor

/azp run python - eventhub - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

benbp
benbp previously approved these changes Nov 11, 2021
@benbp benbp requested review from yunhaoling and benbp November 11, 2021 20:34
@benbp benbp dismissed their stale review November 11, 2021 20:34

Tests skipped

sdk/eventhub/tests.yml Outdated Show resolved Hide resolved
@v-xuto
Copy link
Member Author

v-xuto commented Jan 5, 2022

@yunhaoling @benbp I rerun a new weekly pipeline, test(test_auth.py::test_client_secret_credential) failed in UsGov and China clouds. Error is: Unauthorized access. 'Send' claim(s) are required to perform this operation. For more details please check here. How do we solve the question of permissions Azure Event Hubs Data Sender?

@benbp
Copy link
Member

benbp commented Jan 5, 2022

@v-xuto you may need to add another RBAC policy in the ARM template test-resources.json file? However some quick internet searching indicates that this error may be misleading, and it's actually a missing password? https://stackoverflow.com/a/68833937

I would try adding the claim first in this array and see if that fixes the issue: https://github.com/Azure/azure-sdk-for-python/blob/main/sdk/eventhub/test-resources.json#L189-L215

The role id can be found here and is 2b629674-e913-4c01-ae53-ef4638d8f975.

@v-xuto
Copy link
Member Author

v-xuto commented Jan 6, 2022

@benbp I run this test(test_auth.py::test_client_secret_credential) locally, and I also encounter the same permission problem. So, I debug the test code and set the breakpoint after the eventhub and other resources are created. After the resources are created, I go to the portal to find the corresponding eventhub and add permissions(Azure Event Hubs Data Sender) to it, and continue to run the test. The test result is PASS.
I'm not sure if this will help the same issue we encountered on the weekly pipeline.

@v-xuto
Copy link
Member Author

v-xuto commented Jan 12, 2022

@benbp What is your progress on this issue (Unauthorized access. 'Send' claim(s) are required to perform this operation.)?

@benbp
Copy link
Member

benbp commented Jan 18, 2022

@v-xuto sorry for the delay I was on vacation. Have you tried updating the RBAC assignments in the ARM template similar to the examples I linked?

@v-xuto
Copy link
Member Author

v-xuto commented Jan 19, 2022

@benbp I have tried updating the RBAC assignments in the ARM template, it still reports the same error. For more details please check here.
Through investigation, I think this issue may be because it uses ARM template + PowerShellPreparer to deploy resources, some using MgmtClient while some using ARM template. When using MgmtClient to create services, there is no Azure Event Hubs Data Ownerpermission added here.

@yunhaoling Any ideas about this issue(Unauthorized access. 'Send' claim(s) are required to perform this operation)?

sdk/eventhub/test-resources.json Outdated Show resolved Hide resolved
sdk/eventhub/azure-eventhub/conftest.py Outdated Show resolved Hide resolved
@v-xuto
Copy link
Member Author

v-xuto commented Jan 28, 2022

@benbp @swathipil Now, rerun the Event Hub weekly pipeline, there is a new issue here.

When running tests(test_send.py::test_send_with_partition_key and its async test ) in windows2019_36 on China cloud, test failed and the error message is: uamqp.errors.ConnectionClose: ErrorCodes.UnknownError: Connection in an unexpected error state. For more details please check here.

Do you have any ideas to solve this issue?

@swathipil
Copy link
Member

/azp run python - eventhub - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@swathipil
Copy link
Member

/azp run python - eventhub - tests

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@swathipil
Copy link
Member

/azp run python - eventhub - tests

@swathipil
Copy link
Member

/azp run python - eventhub - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

1 similar comment
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@swathipil
Copy link
Member

green EH tests-weekly pipeline: https://dev.azure.com/azure-sdk/internal/_build/results?buildId=2613023&view=results

@ghost
Copy link

ghost commented Mar 8, 2023

Hello @swathipil!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost
Copy link

ghost commented Mar 8, 2023

Apologies, while this PR appears ready to be merged, I've been configured to only merge when all checks have explicitly passed. The following integrations have not reported any progress on their checks and are blocking auto-merge:

  1. Azure Pipelines

These integrations are possibly never going to report a check, and unblocking auto-merge likely requires a human being to update my configuration to exempt these integrations from requiring a passing check.

Give feedback on this
From the bot dev team

We've tried to tune the bot such that it posts a comment like this only when auto-merge is blocked for exceptional, non-intuitive reasons. When the bot's auto-merge capability is properly configured, auto-merge should operate as you would intuitively expect and you should not see any spurious comments.

Please reach out to us at [email protected] to provide feedback if you believe you're seeing this comment appear spuriously. Please note that we usually are unable to update your bot configuration on your team's behalf, but we're happy to help you identify your bot admin.

@ghost ghost merged commit ab0b377 into Azure:main Mar 8, 2023
ghost pushed a commit that referenced this pull request Mar 25, 2023
This pull request was closed.
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.

6 participants