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.

Sign up for GitHub

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

Pk5/fix integration tests for release #195

Merged
merged 3 commits into from
May 16, 2024
Merged

Pk5/fix integration tests for release #195

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bfe9b7d
Add retry logic to fix integration tests
patrykkulik-microsoft May 15, 2024
fcc45ab
Modify the publisher name because previous name is used up
patrykkulik-microsoft May 15, 2024
6715518
Regenerate the recordings
patrykkulik-microsoft May 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 21 additions & 6 deletions src/aosm/azext_aosm/common/artifact.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
from azext_aosm.common.registry import ContainerRegistry, AzureContainerRegistry
from azext_aosm.vendored_sdks.models import ArtifactType
from azext_aosm.vendored_sdks import HybridNetworkManagementClient
from azure.core.exceptions import ServiceResponseError
from knack.log import get_logger
from oras.client import OrasClient

Expand Down Expand Up @@ -82,12 +83,26 @@ def _manifest_credentials(
aosm_client: HybridNetworkManagementClient,
) -> MutableMapping[str, Any]:
"""Gets the details for uploading the artifacts in the manifest."""
return aosm_client.artifact_manifests.list_credential(
resource_group_name=config.publisherResourceGroupName,
publisher_name=config.publisherName,
artifact_store_name=config.acrArtifactStoreName,
artifact_manifest_name=config.acrManifestName,
).as_dict()
retries = 0
# This retry logic is to handle the ServiceResponseError that is hit in the integration tests.
# This error is not hit when running the cli normally because the CLI framework automatically retries,
# the testing framework does not support automatic retries.
while retries < 2:
Copy link

Choose a reason for hiding this comment

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

Couple of questions:

  • Are automatic retries going to race/conflict with these ones? I'm not sure what would happen if they did - maybe it's fine.
  • Why aren't these retries in the test code if it's a limitation in the test framework?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  • The automatic retires should not race with this one because the ServiceResponseError is only raised when running in a test.
  • I have discussed with the rest of the team about fixing in the testing code and we decided that it would be complicated to figure out (if even possible) so we should just fix it in the main code and chase the CLI team about this problem

try:
credential_dict = aosm_client.artifact_manifests.list_credential(
resource_group_name=config.publisherResourceGroupName,
publisher_name=config.publisherName,
artifact_store_name=config.acrArtifactStoreName,
artifact_manifest_name=config.acrManifestName,
).as_dict()
break
except ServiceResponseError as error:
Copy link

Choose a reason for hiding this comment

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

So I think if you hit an exception that isn't a ServiceResponseError the code will just exit. Is that right? If not, there's an infinite loop.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is just going to fail with another exception but we are not going to catch it

retries += 1
if retries == 2:
logger.debug(error, exc_info=True)
raise ServiceResponseError("Failed to get manifest credentials.")

return credential_dict

@staticmethod
def _get_oras_client(manifest_credentials: MutableMapping[str, Any]) -> OrasClient:
Expand Down
2 changes: 1 addition & 1 deletion ...st/integration_tests/integration_test_mocks/mock_input_templates/cnf_input_template.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"location": "uksouth",
// Name of the Publisher resource you want your definition published to.
// Will be created if it does not exist.
"publisher_name": "automated-cli-tests-nginx-publisher",
"publisher_name": "automated-cli-test-nginx-publisher",
// Optional. Resource group for the Publisher resource.
// Will be created if it does not exist (with a default name if none is supplied).
"publisher_resource_group_name": "{{publisher_resource_group_name}}",
Expand Down
4 changes: 2 additions & 2 deletions ...ntegration_tests/integration_test_mocks/mock_input_templates/cnf_nsd_input_template.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
{
"publisher_name": "automated-cli-tests-nginx-publisher",
"publisher_name": "automated-cli-test-nginx-publisher",
"publisher_resource_group_name": "{{publisher_resource_group_name}}",
"acr_artifact_store_name": "nginx-nsd-acr",
"location": "uksouth",
"network_functions": [
{
"publisher": "automated-cli-tests-nginx-publisher",
"publisher": "automated-cli-test-nginx-publisher",
"publisher_resource_group": "{{publisher_resource_group_name}}",
"name": "nginx-nfdg",
"version": "1.0.0",
Expand Down
6 changes: 3 additions & 3 deletions ...st/integration_tests/integration_test_mocks/mock_input_templates/input_multi_nf_nsd.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
{
"location": "uksouth",
"publisher_name": "automated-cli-tests-ubuntu-publisher",
"publisher_name": "automated-cli-test-ubuntu-publisher",
"publisher_resource_group_name": "cli_test_nsd",
"acr_artifact_store_name": "ubuntu-acr",
"resource_element_templates": [
{
"resource_element_type": "NF",
"properties": {
// The name of the existing publisher for the NSD.
"publisher": "automated-cli-tests-ubuntu-publisher",
"publisher": "automated-cli-test-ubuntu-publisher",
// The resource group that the publisher is hosted in.
"publisher_resource_group": "cli_test_nsd",
// The name of the existing Network Function Definition Group to deploy using this NSD.
Expand All @@ -26,7 +26,7 @@
"resource_element_type": "NF",
"properties": {
// The name of the existing publisher for the NSD.
"publisher": "automated-cli-tests-ubuntu-publisher",
"publisher": "automated-cli-test-ubuntu-publisher",
// The resource group that the publisher is hosted in.
"publisher_resource_group": "cli_test_nsd",
// The name of the existing Network Function Definition Group to deploy using this NSD.
Expand Down
4 changes: 2 additions & 2 deletions ...egration_tests/integration_test_mocks/mock_input_templates/input_multiple_instances.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
{
"location": "uksouth",
"publisher_name": "automated-cli-tests-ubuntu-publisher",
"publisher_name": "automated-cli-test-ubuntu-publisher",
"publisher_resource_group_name": "cli_test_nsd",
"acr_artifact_store_name": "ubuntu-acr",
"resource_element_templates": [
{
"resource_element_type": "NF",
"properties": {
// The name of the existing publisher for the NSD.
"publisher": "automated-cli-tests-ubuntu-publisher",
"publisher": "automated-cli-test-ubuntu-publisher",
// The resource group that the publisher is hosted in.
"publisher_resource_group": "cli_test_nsd",
// The name of the existing Network Function Definition Group to deploy using this NSD.
Expand Down
4 changes: 2 additions & 2 deletions ...latest/integration_tests/integration_test_mocks/mock_input_templates/nsd_core_input.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
{
"location": "uksouth",
"publisher_name": "automated-cli-tests-ubuntu-publisher",
"publisher_name": "automated-cli-test-ubuntu-publisher",
"publisher_resource_group_name": "test_publisher_name",
"acr_artifact_store_name": "ubuntu-acr",
"resource_element_templates": [
{
"resource_element_type": "NF",
"properties": {
// The name of the existing publisher for the NSD.
"publisher": "automated-cli-tests-ubuntu-publisher",
"publisher": "automated-cli-test-ubuntu-publisher",
// The resource group that the publisher is hosted in.
"publisher_resource_group": "test_publisher_name",
// The name of the existing Network Function Definition Group to deploy using this NSD.
Expand Down
2 changes: 1 addition & 1 deletion ...st/integration_tests/integration_test_mocks/mock_input_templates/vnf_input_template.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"location": "uksouth",
// Name of the Publisher resource you want your definition published to.
// Will be created if it does not exist.
"publisher_name": "automated-cli-tests-ubuntu-publisher",
"publisher_name": "automated-cli-test-ubuntu-publisher",
// Optional. Resource group for the Publisher resource.
// Will be created if it does not exist (with a default name if none is supplied).
"publisher_resource_group_name": "{{publisher_resource_group_name}}",
Expand Down
2 changes: 1 addition & 1 deletion ...tests/integration_test_mocks/mock_input_templates/vnf_input_with_sas_token_template.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"location": "uksouth",
// Name of the Publisher resource you want your definition published to.
// Will be created if it does not exist.
"publisher_name": "automated-cli-tests-ubuntu-publisher",
"publisher_name": "automated-cli-test-ubuntu-publisher",
// Optional. Resource group for the Publisher resource.
// Will be created if it does not exist (with a default name if none is supplied).
"publisher_resource_group_name": "{{publisher_resource_group_name}}",
Expand Down
4 changes: 2 additions & 2 deletions ...ntegration_tests/integration_test_mocks/mock_input_templates/vnf_nsd_input_template.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
{
"location": "uksouth",
"publisher_name": "automated-cli-tests-ubuntu-publisher",
"publisher_name": "automated-cli-test-ubuntu-publisher",
"publisher_resource_group_name": "{{publisher_resource_group_name}}",
"acr_artifact_store_name": "ubuntu-acr",
"resource_element_templates": [
{
"resource_element_type": "NF",
"properties": {
// The name of the existing publisher for the NSD.
"publisher": "automated-cli-tests-ubuntu-publisher",
"publisher": "automated-cli-test-ubuntu-publisher",
// The resource group that the publisher is hosted in.
"publisher_resource_group": "{{publisher_resource_group_name}}",
// The name of the existing Network Function Definition Group to deploy using this NSD.
Expand Down
2 changes: 1 addition & 1 deletion ...zext_aosm/tests/latest/integration_tests/nsd_output/test_build/all_deploy.parameters.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"location": "uksouth",
"publisherName": "automated-cli-tests-ubuntu-publisher",
"publisherName": "automated-cli-test-ubuntu-publisher",
"publisherResourceGroupName": "test_publisher_name",
"acrArtifactStoreName": "ubuntu-acr",
"acrManifestName": "ubuntu-nsd-manifest-1-0-0",
Expand Down
2 changes: 1 addition & 1 deletion ...est/integration_tests/nsd_output/test_build_multiple_instances/all_deploy.parameters.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"location": "uksouth",
"publisherName": "automated-cli-tests-ubuntu-publisher",
"publisherName": "automated-cli-test-ubuntu-publisher",
"publisherResourceGroupName": "cli_test_nsd",
"acrArtifactStoreName": "ubuntu-acr",
"acrManifestName": "ubuntu-nsd-manifest-1-0-0",
Expand Down
Loading
Loading