From d1c7b8b05678bdc560ca8dedb2a747f38bcfb00d Mon Sep 17 00:00:00 2001 From: sunnycarter <36891339+sunnycarter@users.noreply.github.com> Date: Thu, 17 Aug 2023 14:16:33 +0100 Subject: [PATCH] Fixes for Windows (#58) * Fix bicep render on Windows * Fixes for Windows * python-static-checks ran --- src/aosm/README.md | 6 +++- src/aosm/azext_aosm/_configuration.py | 30 +++++++------------ src/aosm/azext_aosm/custom.py | 16 +++++++--- src/aosm/azext_aosm/delete/delete.py | 4 ++- src/aosm/azext_aosm/deploy/artifact.py | 22 ++++++++++++-- src/aosm/azext_aosm/deploy/deploy_with_arm.py | 16 ++++------ .../generate_nfd/cnf_nfd_generator.py | 8 ++--- src/aosm/azext_aosm/generate_nsd/nf_ret.py | 10 ++----- .../azext_aosm/generate_nsd/nsd_generator.py | 4 +-- .../tests/latest/recording_processors.py | 4 +-- .../test_aosm_cnf_publish_and_delete.py | 2 +- .../test_aosm_vnf_publish_and_delete.py | 2 +- src/aosm/azext_aosm/tests/latest/test_cnf.py | 8 ++--- src/aosm/azext_aosm/tests/latest/test_nsd.py | 26 +++++----------- src/aosm/azext_aosm/tests/latest/test_vnf.py | 12 ++------ 15 files changed, 78 insertions(+), 92 deletions(-) diff --git a/src/aosm/README.md b/src/aosm/README.md index 507ccfa9676..4b66b16245c 100644 --- a/src/aosm/README.md +++ b/src/aosm/README.md @@ -22,7 +22,7 @@ https://github.com/jddarby/azure-cli-extensions/releases/download/aosm-extension To install, download this wheel and run: `az extension add --source path/to/aosm-0.2.0-py2.py3-none-any.whl` -You must also have helm installed, instructions can be found here: https://helm.sh/docs/intro/install/#through-package-managers +For CNFs you will also need helm and docker installed. See [CNFs](#cnfs) below for details. ## Updating @@ -92,6 +92,10 @@ For NSDs, you will need to have a Resource Group with a deployed Publisher, Arti ### Command examples +#### Before you start +`az login` to login to the Azure CLI. +`az account set --subscription ` to choose the subscription you will work on. + #### NFDs Get help on command arguments diff --git a/src/aosm/azext_aosm/_configuration.py b/src/aosm/azext_aosm/_configuration.py index 58455889b7a..b8d14d68f77 100644 --- a/src/aosm/azext_aosm/_configuration.py +++ b/src/aosm/azext_aosm/_configuration.py @@ -48,7 +48,8 @@ "artifact_name": "Name of the artifact", "file_path": ( "Optional. File path of the artifact you wish to upload from your local disk. " - "Delete if not required." + "Delete if not required. Relative paths are relative to the configuration file." + "On Windows escape any backslash with another backslash." ), "blob_sas_url": ( "Optional. SAS URL of the blob artifact you wish to copy to your Artifact" @@ -68,7 +69,8 @@ ), "helm_package_name": "Name of the Helm package", "path_to_chart": ( - "File path of Helm Chart on local disk. Accepts .tgz, .tar or .tar.gz" + "File path of Helm Chart on local disk. Accepts .tgz, .tar or .tar.gz." + " Use Linux slash (/) file separator even if running on Windows." ), "path_to_mappings": ( "File path of value mappings on local disk where chosen values are replaced " @@ -86,7 +88,7 @@ "image to use for the VM." ), "source_registry_id": ( - "Resource ID of the source acr registry from which to pull the image" + "Resource ID of the source acr registry from which to pull the image." ), "source_registry_namespace": ( "Optional. Namespace of the repository of the source acr registry from which " @@ -153,9 +155,7 @@ def output_directory_for_build(self) -> Path: @property def acr_manifest_names(self) -> List[str]: - """ - The list of ACR manifest names. - """ + """The list of ACR manifest names.""" raise NotImplementedError("Subclass must define property") @@ -343,9 +343,7 @@ def validate(self): @dataclass class NFDRETConfiguration: - """ - The configuration required for an NFDV that you want to include in an NSDV. - """ + """The configuration required for an NFDV that you want to include in an NSDV.""" publisher: str = PUBLISHER_NAME publisher_resource_group: str = PUBLISHER_RESOURCE_GROUP @@ -406,10 +404,8 @@ def build_output_folder_name(self) -> Path: @property def arm_template(self) -> ArtifactConfig: - """ - Return the parameters of the ARM template for this RET to be uploaded as part of - the NSDV. - """ + """Return the parameters of the ARM template for this RET to be uploaded as part of + the NSDV.""" artifact = ArtifactConfig() artifact.artifact_name = f"{self.name.lower()}_nf_artifact" @@ -452,9 +448,7 @@ class NSConfiguration(Configuration): nsdv_description: str = DESCRIPTION_MAP["nsdv_description"] def __post_init__(self): - """ - Covert things to the correct format. - """ + """Covert things to the correct format.""" if self.network_functions and isinstance(self.network_functions[0], dict): nf_ret_list = [ NFDRETConfiguration(**config) for config in self.network_functions @@ -506,9 +500,7 @@ def cg_schema_name(self) -> str: @property def acr_manifest_names(self) -> List[str]: - """ - The list of ACR manifest names for all the NF ARM templates. - """ + """The list of ACR manifest names for all the NF ARM templates.""" return [nf.acr_manifest_name(self.nsd_version) for nf in self.network_functions] diff --git a/src/aosm/azext_aosm/custom.py b/src/aosm/azext_aosm/custom.py index 5b9e28f479c..9a8029937b2 100644 --- a/src/aosm/azext_aosm/custom.py +++ b/src/aosm/azext_aosm/custom.py @@ -103,7 +103,11 @@ def _get_config_from_file(config_file: str, configuration_type: str) -> Configur def _generate_nfd( - definition_type: str, config: NFConfiguration, order_params: bool, interactive: bool, force: bool = False + definition_type: str, + config: NFConfiguration, + order_params: bool, + interactive: bool, + force: bool = False, ): """Generate a Network Function Definition for the given type and config.""" nfd_generator: NFDGenerator @@ -199,7 +203,7 @@ def delete_published_definition( definition_type, config_file, clean=False, - force=False + force=False, ): """ Delete a published definition. @@ -277,7 +281,9 @@ def _generate_config(configuration_type: str, output_file: str = "input.json"): ) -def build_design(cmd, client: HybridNetworkManagementClient, config_file: str, force: bool = False): +def build_design( + cmd, client: HybridNetworkManagementClient, config_file: str, force: bool = False +): """ Build a Network Service Design. @@ -386,7 +392,9 @@ def publish_design( deployer.deploy_nsd_from_bicep() -def _generate_nsd(config: NSConfiguration, api_clients: ApiClients, force: bool = False): +def _generate_nsd( + config: NSConfiguration, api_clients: ApiClients, force: bool = False +): """Generate a Network Service Design for the given config.""" if config: nsd_generator = NSDGenerator(config=config, api_clients=api_clients) diff --git a/src/aosm/azext_aosm/delete/delete.py b/src/aosm/azext_aosm/delete/delete.py index 803d98127c2..ba44d07fc50 100644 --- a/src/aosm/azext_aosm/delete/delete.py +++ b/src/aosm/azext_aosm/delete/delete.py @@ -103,7 +103,9 @@ def delete_nsd(self, clean: bool = False, force: bool = False) -> None: f" {self.config.cg_schema_name}?" ) if clean: - print(f"Because of the --clean flag, the NSDG {self.config.nsdg_name} will also be deleted.") + print( + f"Because of the --clean flag, the NSDG {self.config.nsdg_name} will also be deleted." + ) print("There is no undo. Type 'delete' to confirm") if not input_ack("delete", "Confirm delete:"): print("Not proceeding with delete") diff --git a/src/aosm/azext_aosm/deploy/artifact.py b/src/aosm/azext_aosm/deploy/artifact.py index c6ee00789a8..96f5e9c236a 100644 --- a/src/aosm/azext_aosm/deploy/artifact.py +++ b/src/aosm/azext_aosm/deploy/artifact.py @@ -4,6 +4,7 @@ # pylint: disable=unidiomatic-typecheck """A module to handle interacting with artifacts.""" import subprocess +import shutil from dataclasses import dataclass from typing import List, Union @@ -88,20 +89,35 @@ def _upload_helm_to_acr(self, artifact_config: HelmPackageConfig) -> None: registry_name = registry.replace(".azurecr.io", "") # az acr login --name "registry_name" - login_command = ["az", "acr", "login", "--name", registry_name] + # Note that this uses the user running the CLI's AZ login credentials, not the + # manifest credentials retrieved from the ACR. This isn't ideal, but using the + # manifest credentials caused problems so we are doing this for now. + logger.debug("Logging into %s", registry_name) + login_command = [ + str(shutil.which("az")), + "acr", + "login", + "--name", + registry_name, + ] subprocess.run(login_command, check=True) try: logger.debug("Uploading %s to %s", chart_path, target_registry) # helm push "$chart_path" "$target_registry" - push_command = ["helm", "push", chart_path, target_registry] + push_command = [ + str(shutil.which("helm")), + "push", + chart_path, + target_registry, + ] subprocess.run(push_command, check=True) finally: # If we don't logout from the registry, future Artifact uploads to this ACR # will fail with an UNAUTHORIZED error. There is no az acr logout command, # but it is a wrapper around docker, so a call to docker logout will work. - logout_command = ["docker", "logout", registry] + logout_command = [str(shutil.which("docker")), "logout", registry] subprocess.run(logout_command, check=True) def _upload_to_storage_account(self, artifact_config: ArtifactConfig) -> None: diff --git a/src/aosm/azext_aosm/deploy/deploy_with_arm.py b/src/aosm/azext_aosm/deploy/deploy_with_arm.py index 02240830fff..7d694857422 100644 --- a/src/aosm/azext_aosm/deploy/deploy_with_arm.py +++ b/src/aosm/azext_aosm/deploy/deploy_with_arm.py @@ -146,9 +146,7 @@ def deploy_nfd_from_bicep(self) -> None: print("Done") def _vnfd_artifact_upload(self) -> None: - """ - Uploads the VHD and ARM template artifacts - """ + """Uploads the VHD and ARM template artifacts.""" assert isinstance(self.config, VNFConfiguration) storage_account_manifest = ArtifactManifestOperator( self.config, @@ -176,9 +174,7 @@ def _vnfd_artifact_upload(self) -> None: arm_template_artifact.upload(self.config.arm_template) def _cnfd_artifact_upload(self) -> None: - """ - Uploads the Helm chart and any additional images. - """ + """Uploads the Helm chart and any additional images.""" assert isinstance(self.config, CNFConfiguration) acr_properties = self.api_clients.aosm_client.artifact_stores.get( resource_group_name=self.config.publisher_resource_group_name, @@ -291,7 +287,9 @@ def parameters(self) -> Dict[str, Any]: def construct_parameters(self) -> Dict[str, Any]: """ - Create the parmeters dictionary for vnfdefinitions.bicep. VNF specific. + Create the parmeters dictionary for vnfdefinitions.bicep. + + VNF specific. """ if self.resource_type == VNF: assert isinstance(self.config, VNFConfiguration) @@ -448,9 +446,7 @@ def deploy_nsd_from_bicep(self) -> None: print("Done") def deploy_manifest_template(self) -> None: - """ - Deploy the bicep template defining the manifest. - """ + """Deploy the bicep template defining the manifest.""" print("Deploy bicep template for Artifact manifests") logger.debug("Deploy manifest bicep") diff --git a/src/aosm/azext_aosm/generate_nfd/cnf_nfd_generator.py b/src/aosm/azext_aosm/generate_nfd/cnf_nfd_generator.py index 3a8a688ee46..2c7df29848c 100644 --- a/src/aosm/azext_aosm/generate_nfd/cnf_nfd_generator.py +++ b/src/aosm/azext_aosm/generate_nfd/cnf_nfd_generator.py @@ -43,9 +43,7 @@ @dataclass class Artifact: - """ - Information about an artifact. - """ + """Information about an artifact.""" name: str version: str @@ -319,8 +317,8 @@ def _copy_to_output_directory(self) -> None: """ Copy files from the temp directory to the output directory. - Files are the config mappings, schema and bicep templates (artifact manifest - and NFDV). + Files are the config mappings, schema and bicep templates (artifact manifest and + NFDV). """ assert self._tmp_dir diff --git a/src/aosm/azext_aosm/generate_nsd/nf_ret.py b/src/aosm/azext_aosm/generate_nsd/nf_ret.py index 8a476181d12..ab335b1123f 100644 --- a/src/aosm/azext_aosm/generate_nsd/nf_ret.py +++ b/src/aosm/azext_aosm/generate_nsd/nf_ret.py @@ -19,9 +19,7 @@ class NFRETGenerator: - """ - Represents a single network function resource element template withing an NSD. - """ + """Represents a single network function resource element template withing an NSD.""" def __init__( self, api_clients: ApiClients, config: NFDRETConfiguration, cg_schema_name: str @@ -63,7 +61,7 @@ def _get_nfdv( @property def config_mappings(self) -> Dict[str, Any]: """ - Return the contents of the config mapping file for this RET. + Return the contents of the config mapping file for this RET. Output will look something like: { @@ -129,9 +127,7 @@ def nf_bicep_substitutions(self) -> Dict[str, Any]: @property def config_schema_snippet(self) -> Dict[str, Any]: - """ - Return the CGS snippet for this NF. - """ + """Return the CGS snippet for this NF.""" nfdv_version_description_string = ( f"The version of the {self.config.name} " "NFD to use. This version must be compatible with (have the same " diff --git a/src/aosm/azext_aosm/generate_nsd/nsd_generator.py b/src/aosm/azext_aosm/generate_nsd/nsd_generator.py index 986c26faa71..e1e80740b0f 100644 --- a/src/aosm/azext_aosm/generate_nsd/nsd_generator.py +++ b/src/aosm/azext_aosm/generate_nsd/nsd_generator.py @@ -142,9 +142,7 @@ def _write_config_group_schema_json(self, output_directory) -> None: logger.debug("%s created", schema_path) def _write_config_mapping_files(self, output_directory) -> None: - """ - Write out a config mapping file for each NF. - """ + """Write out a config mapping file for each NF.""" temp_mappings_folder_path = os.path.join( output_directory, CONFIG_MAPPINGS_DIR_NAME ) diff --git a/src/aosm/azext_aosm/tests/latest/recording_processors.py b/src/aosm/azext_aosm/tests/latest/recording_processors.py index e29f06a3cf8..ebc2cdbff11 100644 --- a/src/aosm/azext_aosm/tests/latest/recording_processors.py +++ b/src/aosm/azext_aosm/tests/latest/recording_processors.py @@ -7,9 +7,7 @@ # the recordings so that we can avoid checking in secrets to the repo. # -------------------------------------------------------------------------------------------- -from azure.cli.testsdk.scenario_tests import ( - RecordingProcessor -) +from azure.cli.testsdk.scenario_tests import RecordingProcessor from azure.cli.testsdk.scenario_tests.utilities import is_text_payload import json diff --git a/src/aosm/azext_aosm/tests/latest/test_aosm_cnf_publish_and_delete.py b/src/aosm/azext_aosm/tests/latest/test_aosm_cnf_publish_and_delete.py index 90d0835819f..7012bfe55c0 100644 --- a/src/aosm/azext_aosm/tests/latest/test_aosm_cnf_publish_and_delete.py +++ b/src/aosm/azext_aosm/tests/latest/test_aosm_cnf_publish_and_delete.py @@ -53,7 +53,7 @@ # class CnfNsdTest(ScenarioTest): # @ResourceGroupPreparer() # def test_cnf_nsd_publish_and_delete(self, resource_group): -# # We are overriding a resource group name here because we need to have some +# # We are overriding a resource group name here because we need to have some # # resources predeployed in order to get around the timeout bug in the testing framework. # resource_group = "patrykkulik-test" diff --git a/src/aosm/azext_aosm/tests/latest/test_aosm_vnf_publish_and_delete.py b/src/aosm/azext_aosm/tests/latest/test_aosm_vnf_publish_and_delete.py index 904e4d1f565..1126574241f 100644 --- a/src/aosm/azext_aosm/tests/latest/test_aosm_vnf_publish_and_delete.py +++ b/src/aosm/azext_aosm/tests/latest/test_aosm_vnf_publish_and_delete.py @@ -98,7 +98,7 @@ # f'az aosm nfd build -f "{nfd_input_file_path}" --definition-type vnf --force' # ) -# # There is currently a bug in the CLI testing framework that causes the command +# # There is currently a bug in the CLI testing framework that causes the command # # to fail on timeout. This is a workaround to retry the command if it fails. # retry_attempts = 0 # while retry_attempts < 2: diff --git a/src/aosm/azext_aosm/tests/latest/test_cnf.py b/src/aosm/azext_aosm/tests/latest/test_cnf.py index 05f6ead7c5a..a0137be2346 100644 --- a/src/aosm/azext_aosm/tests/latest/test_cnf.py +++ b/src/aosm/azext_aosm/tests/latest/test_cnf.py @@ -15,9 +15,7 @@ class TestCNF(unittest.TestCase): def test_generate_config(self): - """ - Test generating a config file for a VNF. - """ + """Test generating a config file for a VNF.""" starting_directory = os.getcwd() with TemporaryDirectory() as test_dir: os.chdir(test_dir) @@ -29,9 +27,7 @@ def test_generate_config(self): os.chdir(starting_directory) def test_build(self): - """ - Test the build command for CNFs. - """ + """Test the build command for CNFs.""" starting_directory = os.getcwd() with TemporaryDirectory() as test_dir: os.chdir(test_dir) diff --git a/src/aosm/azext_aosm/tests/latest/test_nsd.py b/src/aosm/azext_aosm/tests/latest/test_nsd.py index cda783c7a50..81379443798 100644 --- a/src/aosm/azext_aosm/tests/latest/test_nsd.py +++ b/src/aosm/azext_aosm/tests/latest/test_nsd.py @@ -134,9 +134,7 @@ def __init__(self) -> None: def validate_schema_against_metaschema(schema_data): - """ - Validate that the schema produced by the CLI matches the AOSM metaschema. - """ + """Validate that the schema produced by the CLI matches the AOSM metaschema.""" # There is a bug in the jsonschema module that means that it hits an error in with # the "$id" bit of the metaschema. Here we use a modified version of the metaschema @@ -151,9 +149,7 @@ def validate_schema_against_metaschema(schema_data): def validate_json_against_schema(json_data, schema_file): - """ - Validate some test data against the schema produced by the CLI. - """ + """Validate some test data against the schema produced by the CLI.""" with open(schema_file, "r", encoding="utf8") as f: schema = json.load(f) @@ -183,7 +179,7 @@ def build_bicep(bicep_template_path): def compare_to_expected_output(expected_folder_name: str): """ - Compares nsd-bicep-templates to the supplied folder name + Compares nsd-bicep-templates to the supplied folder name. :param expected_folder_name: The name of the folder within nsd_output to compare with. @@ -213,9 +209,7 @@ def compare_to_expected_output(expected_folder_name: str): class TestNSDGenerator: def test_generate_config(self): - """ - Test generating a config file for a VNF. - """ + """Test generating a config file for a VNF.""" starting_directory = os.getcwd() with TemporaryDirectory() as test_dir: os.chdir(test_dir) @@ -228,9 +222,7 @@ def test_generate_config(self): @patch("azext_aosm.custom.cf_resources") def test_build(self, cf_resources): - """ - Test building the NSD bicep templates. - """ + """Test building the NSD bicep templates.""" starting_directory = os.getcwd() with TemporaryDirectory() as test_dir: os.chdir(test_dir) @@ -254,9 +246,7 @@ def test_build(self, cf_resources): @patch("azext_aosm.custom.cf_resources") def test_build_multiple_instances(self, cf_resources): - """ - Test building the NSD bicep templates with multiple NFs allowed. - """ + """Test building the NSD bicep templates with multiple NFs allowed.""" starting_directory = os.getcwd() with TemporaryDirectory() as test_dir: os.chdir(test_dir) @@ -280,9 +270,7 @@ def test_build_multiple_instances(self, cf_resources): @patch("azext_aosm.custom.cf_resources") def test_build_multiple_nfs(self, cf_resources): - """ - Test building the NSD bicep templates with multiple NFs allowed. - """ + """Test building the NSD bicep templates with multiple NFs allowed.""" starting_directory = os.getcwd() with TemporaryDirectory() as test_dir: os.chdir(test_dir) diff --git a/src/aosm/azext_aosm/tests/latest/test_vnf.py b/src/aosm/azext_aosm/tests/latest/test_vnf.py index 946255b8ccd..eacc91a4727 100644 --- a/src/aosm/azext_aosm/tests/latest/test_vnf.py +++ b/src/aosm/azext_aosm/tests/latest/test_vnf.py @@ -15,9 +15,7 @@ class TestVNF(unittest.TestCase): def test_generate_config(self): - """ - Test generating a config file for a VNF. - """ + """Test generating a config file for a VNF.""" starting_directory = os.getcwd() with TemporaryDirectory() as test_dir: os.chdir(test_dir) @@ -29,9 +27,7 @@ def test_generate_config(self): os.chdir(starting_directory) def test_build(self): - """ - Test building an NFDV for a VNF. - """ + """Test building an NFDV for a VNF.""" starting_directory = os.getcwd() with TemporaryDirectory() as test_dir: os.chdir(test_dir) @@ -52,9 +48,7 @@ def test_build(self): os.chdir(starting_directory) def test_build_with_ordered_params(self): - """ - Test building an NFDV for a VNF. - """ + """Test building an NFDV for a VNF.""" starting_directory = os.getcwd() with TemporaryDirectory() as test_dir: os.chdir(test_dir)