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

{Keyvault} Fix test failure in az keyvault certificate create #27016

Merged
merged 5 commits into from
Jul 27, 2023

Conversation

evelyn-ys
Copy link
Member

Related command

az keyvault certificate create

Description

Testing Guide

History Notes

[Component Name 1] BREAKING CHANGE: az command a: Make some customer-facing breaking change
[Component Name 2] az command b: Add some customer-facing feature


This checklist is used to make sure that common guidelines for a pull request are followed.

@azure-client-tools-bot-prd
Copy link

azure-client-tools-bot-prd bot commented Jul 27, 2023

️✔️AzureCLI-FullTest
️✔️acr
️✔️2020-09-01-hybrid
️✔️3.10
️✔️3.9
️✔️latest
️✔️3.10
️✔️3.9
️✔️acs
️✔️2020-09-01-hybrid
️✔️3.10
️✔️3.9
️✔️latest
️✔️3.10
️✔️3.9
️✔️advisor
️✔️latest
️✔️3.10
️✔️3.9
️✔️ams
️✔️latest
️✔️3.10
️✔️3.9
️✔️apim
️✔️latest
️✔️3.10
️✔️3.9
️✔️appconfig
️✔️latest
️✔️3.10
️✔️3.9
️✔️appservice
️✔️latest
️✔️3.10
️✔️3.9
️✔️aro
️✔️latest
️✔️3.10
️✔️3.9
️✔️backup
️✔️latest
️✔️3.10
️✔️3.9
️✔️batch
️✔️latest
️✔️3.10
️✔️3.9
️✔️batchai
️✔️latest
️✔️3.10
️✔️3.9
️✔️billing
️✔️latest
️✔️3.10
️✔️3.9
️✔️botservice
️✔️latest
️✔️3.10
️✔️3.9
️✔️cdn
️✔️latest
️✔️3.10
️✔️3.9
️✔️cloud
️✔️latest
️✔️3.10
️✔️3.9
️✔️cognitiveservices
️✔️latest
️✔️3.10
️✔️3.9
️✔️config
️✔️latest
️✔️3.10
️✔️3.9
️✔️configure
️✔️latest
️✔️3.10
️✔️3.9
️✔️consumption
️✔️latest
️✔️3.10
️✔️3.9
️✔️container
️✔️latest
️✔️3.10
️✔️3.9
️✔️core
️✔️2018-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.10
️✔️3.9
️✔️latest
️✔️3.10
️✔️3.9
️✔️cosmosdb
️✔️latest
️✔️3.10
️✔️3.9
️✔️databoxedge
️✔️2019-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.10
️✔️3.9
️✔️latest
️✔️3.10
️✔️3.9
️✔️dla
️✔️latest
️✔️3.10
️✔️3.9
️✔️dls
️✔️latest
️✔️3.10
️✔️3.9
️✔️dms
️✔️latest
️✔️3.10
️✔️3.9
️✔️eventgrid
️✔️latest
️✔️3.10
️✔️3.9
️✔️eventhubs
️✔️latest
️✔️3.10
️✔️3.9
️✔️feedback
️✔️latest
️✔️3.10
️✔️3.9
️✔️find
️✔️latest
️✔️3.10
️✔️3.9
️✔️hdinsight
️✔️latest
️✔️3.10
️✔️3.9
️✔️identity
️✔️latest
️✔️3.10
️✔️3.9
️✔️iot
️✔️2019-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.10
️✔️3.9
️✔️latest
️✔️3.10
️✔️3.9
️✔️keyvault
️✔️2018-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.10
️✔️3.9
️✔️latest
️✔️3.10
️✔️3.9
️✔️kusto
️✔️latest
️✔️3.10
️✔️3.9
️✔️lab
️✔️latest
️✔️3.10
️✔️3.9
️✔️managedservices
️✔️latest
️✔️3.10
️✔️3.9
️✔️maps
️✔️latest
️✔️3.10
️✔️3.9
️✔️marketplaceordering
️✔️latest
️✔️3.10
️✔️3.9
️✔️monitor
️✔️latest
️✔️3.10
️✔️3.9
️✔️mysql
️✔️latest
️✔️3.10
️✔️3.9
️✔️netappfiles
️✔️latest
️✔️3.10
️✔️3.9
️✔️network
️✔️2018-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️latest
️✔️3.10
️✔️3.9
️✔️policyinsights
️✔️latest
️✔️3.10
️✔️3.9
️✔️privatedns
️✔️latest
️✔️3.10
️✔️3.9
️✔️profile
️✔️latest
️✔️3.10
️✔️3.9
️✔️rdbms
️✔️latest
️✔️3.10
️✔️3.9
️✔️redis
️✔️latest
️✔️3.10
️✔️3.9
️✔️relay
️✔️latest
️✔️3.10
️✔️3.9
️✔️resource
️✔️2018-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️latest
️✔️3.10
️✔️3.9
️✔️role
️✔️latest
️✔️3.10
️✔️3.9
️✔️search
️✔️latest
️✔️3.10
️✔️3.9
️✔️security
️✔️latest
️✔️3.10
️✔️3.9
️✔️servicebus
️✔️latest
️✔️3.10
️✔️3.9
️✔️serviceconnector
️✔️latest
️✔️3.10
️✔️3.9
️✔️servicefabric
️✔️latest
️✔️3.10
️✔️3.9
️✔️signalr
️✔️latest
️✔️3.10
️✔️3.9
️✔️sql
️✔️latest
️✔️3.10
️✔️3.9
️✔️sqlvm
️✔️latest
️✔️3.10
️✔️3.9
️✔️storage
️✔️2018-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.10
️✔️3.9
️✔️latest
️✔️3.10
️✔️3.9
️✔️synapse
️✔️latest
️✔️3.10
️✔️3.9
️✔️telemetry
️✔️2018-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.10
️✔️3.9
️✔️latest
️✔️3.10
️✔️3.9
️✔️util
️✔️latest
️✔️3.10
️✔️3.9
️✔️vm
️✔️2018-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.10
️✔️3.9
️✔️latest
️✔️3.10
️✔️3.9

@azure-client-tools-bot-prd
Copy link

azure-client-tools-bot-prd bot commented Jul 27, 2023

️✔️AzureCLI-BreakingChangeTest
️✔️Non Breaking Changes

@ghost ghost added the Auto-Assign Auto assign by bot label Jul 27, 2023
@yonzhan
Copy link
Collaborator

yonzhan commented Jul 27, 2023

Keyvault

@ghost ghost requested a review from yonzhan July 27, 2023 03:16
@ghost ghost assigned evelyn-ys Jul 27, 2023
@ghost ghost added the KeyVault az keyvault label Jul 27, 2023
@evelyn-ys evelyn-ys marked this pull request as draft July 27, 2023 03:22
@evelyn-ys evelyn-ys merged commit 8ac19ca into Azure:dev Jul 27, 2023
55 checks passed
@jiasli
Copy link
Member

jiasli commented Jul 27, 2023

To troubleshoot multithreading issues, let pytest print the thread info:

pytest --log-level=DEBUG --log-format "%(levelname)s %(name)s %(thread)d %(threadName)s %(message)s" src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py::KeyVaultCertificateRestoreScenarioTest::test_keyvault_certificate_soft_delete

The log shows both MainThread and LROPoller are consuming the same URL https://cli-test-kv-ct-sd-000002.vault.azure.net/certificates/cert1/pending?api-version=REDACTED and waiting for a response with "status":"completed":

INFO azure.core.pipeline.policies.http_logging_policy 505396 LROPoller(c39dc85b-cc1d-4a9e-876f-26abd6785abd) Request URL: 'https://cli-test-kv-ct-sd-000002.vault.azure.net/certificates/cert1/pending?api-version=REDACTED'
Request method: 'GET'

INFO azure.core.pipeline.policies.http_logging_policy 466100 MainThread Request URL: 'https://cli-test-kv-ct-sd-000002.vault.azure.net/certificates/cert1/pending?api-version=REDACTED'
Request method: 'GET'

az keyvault certificate create command calls client.begin_create_certificate that returns an LROPoller:

client.begin_create_certificate(
certificate_name=certificate_name, policy=policy, enabled=not disabled, tags=tags)

The LROPoller starts a new thread LROPoller that checks the operation result, but the poller is neither returned to CLI core for progress bar, nor waited with .result().

The MainThread also checks the operation result:

check = client.get_certificate_operation(certificate_name)

Conflict occurs because the recording src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/recordings/test_keyvault_certificate_soft_delete.yaml only has one entry with "status":"completed".

  • If MainThread runs faster, get_certificate_operation in LROPoller thread will not have enough recording entries to consume and fail, but the error won't surface because the result of the poller is not checked.
  • If LROPoller thread runs faster, get_certificate_operation in MainThread thread will not have enough recording entries to consume and fail. An error will be thrown.

@@ -259,7 +257,7 @@ interactions:
uri: https://cli-test-kv-ct-sd-000002.vault.azure.net/certificates/cert1/pending?api-version=7.4
response:
body:
string: '{"id":"https://cli-test-kv-ct-sd-000002.vault.azure.net/certificates/cert1/pending","issuer":{"name":"Self"},"csr":"MIIDFjCCAf4CAQAwdTEdMBsGA1UEAxMUd3d3Lm15dGVzdGRvbWFpbi5jb20xEzARBgNVBAsTClRlc3ROdWdnZXQxFDASBgNVBAoTC1Rlc3QgTm9vZGxlMQ8wDQYDVQQHEwZSZWRtb24xCzAJBgNVBAgTAldBMQswCQYDVQQGEwJVUzCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAOBjCOg4lKf1niwEhftBtaQH+YgxrqGy7HsiFW8TsDJNBXEC08qKtL0KFNyqVtJ2NPNOwQoq32cq8qZAorYt/PIPhxXo0DQU8K88roVOk042B26F2auitu5OqwgFniseQsYasCCrhLPOGSGdDG6TO2SXc0ANSCkg0de7w64htMfn1bNvG9AsfpZZ8dyoUg9nfpwOtu8VvWqd5T0hi3lbIr8rQJdeaZl+iuFwcp+QDt/OH7mo5eAjjxAJNeLo0I7tmULq3DBiiSq1QtEJAwhgyJW90QV9tFDIsNkfF2NRALhpD/TCJpUJwnqHTycanCRqetPWtTiWu+YLsv1oavBWKz0CAwEAAaBcMFoGCSqGSIb3DQEJDjFNMEswDgYDVR0PAQH/BAQDAgLsMB0GA1UdJQQWMBQGCCsGAQUFBwMBBggrBgEFBQcDAjAPBgNVHREECDAGggExggEyMAkGA1UdEwQCMAAwDQYJKoZIhvcNAQELBQADggEBAK3MV4U7gKiibw4hlmFIhaPYKAu4+vCEiS4tBKNr+77IxVgFH037bXC23fTgGWyCX7xrrgMU7yXrZKuTNz+hBu/5fi1mdlMDbFJfqjUCr3jqB+mYhdzZ96K/RvniJejC7lLpE7KTd3IuBndcbAJItvR4F8KogGN7zs/B1PQdm99TVcrApeZbA0LiZlg5/r0Eexf/LgsoAPu1qHWv/ryK6YAuI8ImJ3iwoRIudldoPQEQy1Y5p3MJZOthDpi149MLio3IM07vKywZX9viVvgDYb1xHpGuTOHAqIP9n8GzHtnCRSOg61xXQyv/bsn2V6ZiLolipVR98+38/sLgsIXMgnM=","cancellation_requested":false,"status":"completed","target":"https://cli-test-kv-ct-sd-000002.vault.azure.net/certificates/cert1","request_id":"e0775da185a5408bbd30f3bfe40d9b33"}'
string: '{"id":"https://cli-test-kv-ct-sd-000002.vault.azure.net/certificates/cert1/pending","issuer":{"name":"Self"},"csr":"MIIDFjCCAf4CAQAwdTEdMBsGA1UEAxMUd3d3Lm15dGVzdGRvbWFpbi5jb20xEzARBgNVBAsTClRlc3ROdWdnZXQxFDASBgNVBAoTC1Rlc3QgTm9vZGxlMQ8wDQYDVQQHEwZSZWRtb24xCzAJBgNVBAgTAldBMQswCQYDVQQGEwJVUzCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAK3jtchQr0nSlBSKlaKMN+ETENMd9QfvbE3rXOymj6VkLcYexkHdCUrB0C64xUTWZDb39iP9MDKRgHJU/lY5s701/CHP734YihoTbr6f+i1BqZydYYDA0oYA3dfVLhkEkHUYrJdPK/1i3KEqD57QpeB3zluEUNGST7GpsS0Qdd8Doq02gEejEisNqwSABNpar3A4RwxCIQ/TP+V2J5RCRS1ZwQgQgstaTCxqz7CcFfKOdu0/ecLszV3xpHIsvZp0I9qBn0yNxekES/IPnNDqXBsYnNV2yqJJKaTcFpURfleQvW8+SIrr/0aVKAgA2PDhvUWopHwSsl+LumrdfYE0k4kCAwEAAaBcMFoGCSqGSIb3DQEJDjFNMEswDgYDVR0PAQH/BAQDAgLsMB0GA1UdJQQWMBQGCCsGAQUFBwMBBggrBgEFBQcDAjAPBgNVHREECDAGggExggEyMAkGA1UdEwQCMAAwDQYJKoZIhvcNAQELBQADggEBAITTgptG9aJAjAKT5gj8nXnEIaBDq8o2KNrGnuNdJpF28kAYTyqRWsXYV8YqCSNu+wlQhGcu4JAnSC69155t0i5iKt9RuWFxioWEyZdlrOMwK+h6S7XIpucOGckhANb5mXEmVYXT897ICfb7q/10NthCRkMYeKcI+j64XgmEXVSONPUK77Z3NsXpLUN6HkZ1u7dog0HN3m6aNoVaCWUbBKVWMNxrZJm94h21JJGQK1ZyLD3AKYuwRHdDAFCUQlo+qN6kWBE/SsAHXbYF4PWQQmyIAZO4ZHmB/tE1LNypWtZQqn8Dtol6nez/zhWJDoUOYg9dPIBhIbyii4xHwssJcZo=","cancellation_requested":false,"status":"completed","target":"https://cli-test-kv-ct-sd-000002.vault.azure.net/certificates/cert1","request_id":"cc5cdd9c09324a6bbda8013c24394c95"}'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are 2 responses with "status":"completed" now. This is one.

uri: https://cli-test-kv-ct-sd-000002.vault.azure.net/certificates/cert1/pending?api-version=7.4
response:
body:
string: '{"id":"https://cli-test-kv-ct-sd-000002.vault.azure.net/certificates/cert1/pending","issuer":{"name":"Self"},"csr":"MIIDFjCCAf4CAQAwdTEdMBsGA1UEAxMUd3d3Lm15dGVzdGRvbWFpbi5jb20xEzARBgNVBAsTClRlc3ROdWdnZXQxFDASBgNVBAoTC1Rlc3QgTm9vZGxlMQ8wDQYDVQQHEwZSZWRtb24xCzAJBgNVBAgTAldBMQswCQYDVQQGEwJVUzCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAK3jtchQr0nSlBSKlaKMN+ETENMd9QfvbE3rXOymj6VkLcYexkHdCUrB0C64xUTWZDb39iP9MDKRgHJU/lY5s701/CHP734YihoTbr6f+i1BqZydYYDA0oYA3dfVLhkEkHUYrJdPK/1i3KEqD57QpeB3zluEUNGST7GpsS0Qdd8Doq02gEejEisNqwSABNpar3A4RwxCIQ/TP+V2J5RCRS1ZwQgQgstaTCxqz7CcFfKOdu0/ecLszV3xpHIsvZp0I9qBn0yNxekES/IPnNDqXBsYnNV2yqJJKaTcFpURfleQvW8+SIrr/0aVKAgA2PDhvUWopHwSsl+LumrdfYE0k4kCAwEAAaBcMFoGCSqGSIb3DQEJDjFNMEswDgYDVR0PAQH/BAQDAgLsMB0GA1UdJQQWMBQGCCsGAQUFBwMBBggrBgEFBQcDAjAPBgNVHREECDAGggExggEyMAkGA1UdEwQCMAAwDQYJKoZIhvcNAQELBQADggEBAITTgptG9aJAjAKT5gj8nXnEIaBDq8o2KNrGnuNdJpF28kAYTyqRWsXYV8YqCSNu+wlQhGcu4JAnSC69155t0i5iKt9RuWFxioWEyZdlrOMwK+h6S7XIpucOGckhANb5mXEmVYXT897ICfb7q/10NthCRkMYeKcI+j64XgmEXVSONPUK77Z3NsXpLUN6HkZ1u7dog0HN3m6aNoVaCWUbBKVWMNxrZJm94h21JJGQK1ZyLD3AKYuwRHdDAFCUQlo+qN6kWBE/SsAHXbYF4PWQQmyIAZO4ZHmB/tE1LNypWtZQqn8Dtol6nez/zhWJDoUOYg9dPIBhIbyii4xHwssJcZo=","cancellation_requested":false,"status":"completed","target":"https://cli-test-kv-ct-sd-000002.vault.azure.net/certificates/cert1","request_id":"cc5cdd9c09324a6bbda8013c24394c95"}'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is another one "status":"completed".

avgale pushed a commit to avgale/azure-cli that referenced this pull request Aug 24, 2023
…e#27016)

* Fix certificate create

* clean code

* recover original logic

* refine

* fix linter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Auto-Assign Auto assign by bot KeyVault az keyvault
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants