Add aosm extension #6426
Add aosm extension #6426
Conversation
* Add temporary build workflow for AOSM extension * Add Releaser to maintain a release with the latest build in a consistent place
…extensions into jl/add-cnf-generation
Add empty init files to inner modules so setuptools recognises them
Use latest RG model to remove unnecessary dependency on version
Use latest Deployment model to remove unnecessary dependency on version
…extensions into jl/add-cnf-generation
…extensions into jl/add-cnf-generation
Update readme with install/bug reporting instructions
Add CNF NFD generation to build command
Image value mappings fixes
* Regen Python SDK from 2023-09-01 API - also uses latest AutoRest client - fix for HybridNetworkManagementClient init signature (swap order of subscription_id and credential parameters) * Update CLI extension code to use new SDK * added SAMI to publisher pre deploy * Update bicep templates to use 2023-09-01 * Update NF templates * Update metaschema * Add Allow-Publisher to required feature flags * Use secure objects for deployment parameters
.flake8
Outdated
| E501, # line too long, it is covered by pylint | ||
| E722, # bare except, bad practice, to be removed in the future | ||
| F401, # imported but unused, too many violations, to be removed in the future | ||
| F811, # redefinition of unused, to be removed in the future | ||
| C901 # code flow is too complex, too many violations, to be removed in the future | ||
| W503 # line break before binary operator effect on readability is subjective | ||
| W504 # line break after binary operator effect on readability is subjective | ||
| # line too long, it is covered by pylint | ||
| E501, | ||
| # bare except, bad practice, to be removed in the future | ||
| E722, | ||
| # imported but unused, too many violations, to be removed in the future | ||
| F401, | ||
| # redefinition of unused, to be removed in the future | ||
| F811, | ||
| # code flow is too complex, too many violations, to be removed in the future | ||
| C901, | ||
| # line break before binary operator effect on readability is subjective | ||
| W503, | ||
| # line break after binary operator effect on readability is subjective |
There was a problem hiding this comment.
Why do you need to modify this file?
There was a problem hiding this comment.
The linter complained about the existing format (inline comments and missing commas), so it was modified to keep the linter happy. But you're right, this isn't our file to worry about, so I've reverted it to how it was.
src/aosm/azext_aosm/delete/delete.py
Outdated
| network_function_definition_version_name=self.config.version, | ||
| ) | ||
| LongRunningOperation(self.cli_ctx, "Deleting NFDV...")(poller) | ||
| print("Deleted NFDV.") |
There was a problem hiding this comment.
In fact, the design of the vast majority of CLI delete commands defaults to successful deletion if no information is output. Therefore, it is recommended to keep the design style consistent with other CLI commands.
There was a problem hiding this comment.
I've moved the message to logger.info(), so won't be output by default. Thanks for the advice.
| message = ( | ||
| f"Delete NSDV {self.config.nsd_version} from group" | ||
| f" {self.config.nsd_name} and publisher {self.config.publisher_name}" | ||
| ) | ||
| logger.debug(message) |
There was a problem hiding this comment.
Why print and record debug logs simultaneously?
There was a problem hiding this comment.
We need to rethink how we output to console. These changes are too widespread to make before GA release, so I've opened work item https://dev.azure.com/msazuredev/AzureForOperators/_workitems/edit/992775 to track.
src/aosm/azext_aosm/delete/delete.py
Outdated
| network_service_design_version_name=self.config.nsd_version, | ||
| ) | ||
| LongRunningOperation(self.cli_ctx, "Deleting NSDV...")(poller) | ||
| print("Deleted NSDV.") |
There was a problem hiding this comment.
There was a problem hiding this comment.
I've moved the message to logger.info(), so won't be output by default. Thanks for the advice.
| logger.debug(message) | ||
| print(message) |
There was a problem hiding this comment.
There was a problem hiding this comment.
We need to rethink how we output to console. These changes are too widespread to make before GA release, so I've opened work item https://dev.azure.com/msazuredev/AzureForOperators/_workitems/edit/992775 to track.
| logger.debug(message) | ||
| print(message) |
There was a problem hiding this comment.
Same as above. There are many similar places below, so I won't repeat the comments.
There was a problem hiding this comment.
We need to rethink how we output to console. These changes are too widespread to make before GA release, so I've opened work item https://dev.azure.com/msazuredev/AzureForOperators/_workitems/edit/992775 to track.
src/aosm/azext_aosm/delete/delete.py
Outdated
| ) | ||
| ) | ||
| LongRunningOperation(self.cli_ctx, "Deleting NSD...")(poller) | ||
| print("Deleted NSD") |
There was a problem hiding this comment.
Same as above. There are many similar places below, so I won't repeat the comments.
There was a problem hiding this comment.
I've moved all similar messages to logger.info(), so won't be output by default. Thanks for the advice.
| "License :: OSI Approved :: MIT License", | ||
| ] | ||
|
|
||
| DEPENDENCIES = ["oras~=0.1.19", "azure-storage-blob>=12.15.0", "jinja2>=3.1.2"] |
There was a problem hiding this comment.
Just out of curiosity, can the dependency azure-storage-blob be included by the dependency azure-multiapi-storage and azure-storage-common in the azure-cli-core? code link
There was a problem hiding this comment.
azure-storage-blob depends on azure-storage-common (not vice versa).
https://pypi.org/project/azure-multiapi-storage/ suggests azure-multiapi-storage might cover azure-storage-blob, but I don't know how to use it, and would rather not make an important dependency change at this late stage. I've raised work item https://dev.azure.com/msazuredev/AzureForOperators/_workitems/edit/992831 to track.
|
Due to the super huge size of this PR, and CLI team only reviews the code specification and is not responsible for reviewing the business logic, please have at least one engineer from your team review and sign off this PR |
Correctly get array item types in CNF deployment parameter schemas
Co-authored-by: Xing Zhou <[email protected]>
Thanks for your review. The business logic has been developed over the last few months, with every merge into our development branch reviewed by at least one member of the team. Several of us have also done a general high level review before this PR, and we can sign off the business logic as fully reviewed. |
* moved blob url to vhd config only; untested * stopped error in post innit before validate * changed ordering of inputs so that blob and filepath are next to each other; helptext for filepath different for each option * Markups from sunny * appease mypy --------- Co-authored-by: Jordan <[email protected]> Co-authored-by: Sunny Carter <[email protected]>
|
[Release] Update index.json for extension [ aosm ] : https://dev.azure.com/azclitools/internal/_build/results?buildId=99338&view=results |
| class VNFConfiguration(NFConfiguration): | ||
| blob_artifact_store_name: str = "" | ||
| image_name_parameter: str = "" | ||
| arm_template: Union[Dict[str, str], ArtifactConfig] = ArtifactConfig() |
There was a problem hiding this comment.
It looks like this line is not compatible with 3.11.
Change field default mutability check, allowing only defaults which are hashable instead of any object which is not an instance of dict, list or set. (Contributed by Eric V. Smith in bpo-44674.) --What’s New In Python 3.11 — Python 3.12.0 documentation
This error is raised when run az self-test --debug:
File "/opt/az/azcliextensions/aosm/azext_aosm/_configuration.py", line 267, in <module>
@dataclass
^^^^^^^^^
File "/opt/hostedtoolcache/Python/3.11.6/x64/lib/python3.11/dataclasses.py", line 1230, in dataclass
return wrap(cls)
^^^^^^^^^
File "/opt/hostedtoolcache/Python/3.11.6/x64/lib/python3.11/dataclasses.py", line 1220, in wrap
return _process_class(cls, init, repr, eq, order, unsafe_hash,
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/opt/hostedtoolcache/Python/3.11.6/x64/lib/python3.11/dataclasses.py", line 958, in _process_class
cls_fields.append(_get_field(cls, name, type, kw_only))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/opt/hostedtoolcache/Python/3.11.6/x64/lib/python3.11/dataclasses.py", line 815, in _get_field
raise ValueError(f'mutable default {type(f.default)} for field '
ValueError: mutable default <class 'azext_aosm._configuration.ArtifactConfig'> for field arm_template is not allowed: use default_factory
There was a problem hiding this comment.
Are we expected to support Python 3.11? I can't run azdev setup against 3.11, and the pipelines for our PR didn't test at Python 3.11, as far as I can see.
I have a potential fix, but because of the above problems, testing and ensuring there aren't other occurrences of this problem (I expect there will be), is not possible.
Summary
Work item https://msazure.visualstudio.com/One/_workitems/edit/17617678 on AZ CLI team for the CLI Engagement Onboard.
az aosmcommands for Azure Operator Service Manager.GA Launch Date: 2023-09-30
This is our first Az CLI release and we intend to release the CLI with version <1.0.0.
This is the first time that our team has gone through the APEX process so please can you lay out the next steps after this review so that we can make sure we understand correctly?
Our swagger definition:
We have had an initial meeting with Damien Caro where we explained that standard CRUD operations on many of the HybridNetwork resources are not very beneficial to the user, as it is more likely that they will want to do a suite of complex operations involving multiple linked resource types. As such, have written a customization for the Azure CLI, as an extension
az aosmwhich will help them do this. This does NOT include every CRUD operation you can do over the API, Peter Whiting from AOSM team has been in discussions with the Jeremy Li in the CLI team about this.I am asking AZ CLI team to make a second review of this extension. We want to submit the PR finally 15th September 2023.
Kind Regards and thanks in advance,
Sunny Carter
More detail
The readme outlines the commands, let me expand a little more on the design.
The basic overview of the function is explained here: azure-cli-extensions/README.md . To add more detail to that, our customization would:
Output one or more bicep templates which would create the most complex resources in our API
Allow the user to review those templates
When the user chooses to publish, create some of the simpler HybridNetwork resources using the generated Python SDK. These being:
Then deploy the bicep templates for the more complex HybridNetwork resources
This checklist is used to make sure that common guidelines for a pull request are followed.
Related command
General Guidelines
azdev style <YOUR_EXT>locally? (pip install azdevrequired)Yes
python scripts/ci/test_index.py -qlocally?Yes. output is
Tests:
See src/aosm/development.md
We have Unit Tests and Integration tests. The integration tests seem to be failing with an intermittent issue where the the test is looking for a request that exist in the recording of a test multiple times while it was only run once in the live test
This issue is intermittent but can be fixed by running a test live or allowing playback repeats in the cassette. We did not have enough time to test those solutions in our branch.
I have spoken to Zelin Wang from the Azure CLI team about this and he confirmed that this was an issue that they have seen before too. Here are his recommendations:
_
_
For new extensions:
About Extension Publish
There is a pipeline to automatically build, upload and publish extension wheels.
Once your pull request is merged into main branch, a new pull request will be created to update
src/index.jsonautomatically.You only need to update the version information in file setup.py and historical information in file HISTORY.rst in your PR but do not modify
src/index.json.