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

azure_rm_appgateway.py: Support Version 2 SKUs #198

Merged
merged 3 commits into from
Aug 5, 2020
Merged

azure_rm_appgateway.py: Support Version 2 SKUs #198

merged 3 commits into from
Aug 5, 2020

Conversation

craigforr
Copy link
Contributor

SUMMARY

Add support for Standard_v2 and WAF_v2 SKUs in azure_rm_appgateway.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

azure_rm_appgateway

ADDITIONAL INFORMATION

@craigforr
Copy link
Contributor Author

@haiyuazhang: It looks like you have reviewed recent pull requests.... Is there anything else I need to do here to have this reviewed? It's a pretty simple addition of SKUs. Looking at the history of commits for this module, @gavinfish and @Fred-sun were committers, so if I require additional reviewers, perhaps they would be good candidates.

@Fred-sun
Copy link
Collaborator

@craigforr Haiyuan is on vacation. When he returns, I will ping him to review. Thank you very much!

@craigforr
Copy link
Contributor Author

@Fred-sun: Is @haiyuazhang back yet, or is there someone else who could review this PR ?

@Fred-sun
Copy link
Collaborator

Fred-sun commented Aug 4, 2020

@Fred-sun: Is @haiyuazhang back yet, or is there someone else who could review this PR ?

He has come back and will review this PR today or tomorrow, thank you!

@Fred-sun
Copy link
Collaborator

Fred-sun commented Aug 4, 2020

@craigforr Can you add test cases for the two parameters? ---ansible_collections_azure/tests/integration/targets/azure_rm_appgateway/tasks/main.yml. I test locally and prompt Fail. Can you try with these two parameters? Thank you!

    "msg": "Error creating the Application Gateway instance: Azure Error: ApplicationGatewayFeatureCannotBeEnabledForSelectedSku\nMessage: Application Gateway /subscriptions/xxxxxxxxxxx/resourceGroups/v-xisuRG/providers/Microsoft.Network/applicationGateways/appgatewayfred02 does not support AuthenticationCertificates for the selected SKU tier WAF_v2. Supported SKU tiers are Standard,WAF."

@craigforr
Copy link
Contributor Author

@Fred-sun: I'd be happy to add tests. However, since the v2 SKUs do not support AuthenticationCertificates, as the error you received shows, I believe we will need to create a separate test for these SKUs. Since I'm not familiar with your testing setup, I will need some guidance as to how you typically handle this kind of situation. Should I create a second playbook, next to main.yml in tests/integration/targets/azure_rm_appgateway/tasks/ -- for example, skus_v2.yml -- in order to run these tests ?

@Fred-sun
Copy link
Collaborator

Fred-sun commented Aug 5, 2020

@craigforr Yes, You can recreate an new appgateway in the current test sample ( name: "appgateway{{ rpfx }}02" , update the SKU parameter, and delete it at the end of the test. Thank you very much!

@haiyuazhang haiyuazhang merged commit d3fbf7e into ansible-collections:dev Aug 5, 2020
@haiyuazhang
Copy link
Contributor

I've test it, works。

Fred-sun pushed a commit to Fred-sun/ansible_collections_azure that referenced this pull request Sep 3, 2020
… of a Storage Account. (ansible-collections#108)

* Add support for managing the 'Firewall and virtual networks' settings of
a Storage Account.

Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>

Update azure_rm_publicipaddress

add test sample

codebase cleanup: update format (ansible-collections#131)

* Update format

refine azure devops pipeline (ansible-collections#138)

* refine azure devops pipeline

enable single module test (ansible-collections#144)

*  enable single module test

bug fixing in azure_rm_aks module(ansible-collections#170)

* bug fixing in azure_rm_aks module

For Test( 2.9 to 2.10) (ansible-collections#140)

* adding more testing dimensions

Add FileEndpoint to azure_rm_storageaccount_info (ansible-collections#102)

* Add FileEndpoint to azure_rm_storageaccount_info

Use hasattr instead of dir (ansible-collections#75)

Add ephemeral OS disk for azure_rm_virtualmachine (ansible-collections#124)

* Add ephemeral OS disk  support for azure_rm_virtualmachine

Add ephemeral OS disk for azure_rm_virtualmachinescaleset (ansible-collections#128)

* Add ephemeral OS disk support for azure_rm_virtualmachinescaleset

Change network_client api_version to match latest default api_version (ansible-collections#157)

* #Fixes 156
Change network_client api_version to match latest default api_version

* #Fixes 156
Update azure-mgmt-network to 10.2.0
Update 'latest' version listing and default version for Network Client to 2018-08-01

Add ability to remove all Subnet Service Endpoints when supplying an empty list. (ansible-collections#148)

New azure_rm_privatednszone module with tests (ansible-collections#122)

* New azure_rm_privatednszone module with tests

Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>
Co-authored-by: Fred-sun <xiuxi.sun@qq.com>

Add new azure active directory related modules (ansible-collections#179)

* add ad related modules

* release v0.2.0 preparation and minor ad module bug fixing

fixing sanity testing errors in ad related modules (ansible-collections#182)

* disable ad relate module test
* fixing sanity testing errors in ad related modules

Fixing issues in azure_rm_deployment_info.py  (ansible-collections#180)

- Fixing getting the template_link when it does not exist.
- Feature: Return the correlation_id

Co-authored-by: Steve Kieu <steve.kieu@xvt.com.au>

azure_rm_virtualmachine: Protect against no diskSizeGB (ansible-collections#185)

Disks `diskSizeGB` is not reported if the Virtual Machine is
deallocated.  Trying to fetch it is causing a `KeyError`. This guards
against that KeyError and then nulls out the key so it isn't set
incorrectly in the update call.

Fix wrong module deprecate statement format (ansible-collections#176)

* fix wrong module deprecate statement format

update doc

add batch upload feature in azure_rm_storageblob (ansible-collections#203)

Co-authored-by: haiyuazhang <haiyuan.zhang1982@gmaile.com>

fixing status code issue in azure_rm_deployment (ansible-collections#204)

Co-authored-by: haiyuazhang <haiyuan.zhang1982@gmaile.com>

fixing update check bug in azure_rm_adserviceprincipal (ansible-collections#205)

Bump version to v0.3.0

update doc

change service_url to service_uri (ansible-collections#212)

Storageaccount tlsversion (ansible-collections#207)

* add support for minimum_tls_version

fix(vm): fix boot diagnistics option "no" caused an error (ansible-collections#200)

Fix the issue ansible-collections#158 (ansible-collections#214)

* fix issue ansible-collections#158 and add tests

azure_rm_appgateway.py: Support Version 2 SKUs (ansible-collections#198)

fix sanity test issues (ansible-collections#223)

Bump version to v0.4.0

lift v0.4.0 to v1.0.0

update new pr-pipeline (ansible-collections#229)

Add execution environment metadata (ansible-collections#220)

Storage account allow blob public access parameter (ansible-collections#219)

* Storage account allow blob public access parameter

* Change name of test to match correct attribute.

Update azure_rm_virtualmachine boot_diagnostics resource group config… (ansible-collections#208)

* Update azure_rm_virtualmachine boot_diagnostics resource group configuration

Update azure_rm_storageaccount.py file (ansible-collections#233)

* Uneatable parameters do not need to set default values

fix the problem of disk lun self-increment (ansible-collections#237)

Fixing rule_type reference (ansible-collections#99)

Reference to ev instead of item is preventing the expected rule_type values from functioning

Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>

add load_balancer_sku option for aks (ansible-collections#199)

* added load_balancer_sku option for aks

Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>

Improve OS detection when VM has no osProfile (ansible-collections#197)

We have VM without OS profile (checked on https://resources.azure.com/) and they appear as "unknown" os :-/

This appear to be a known issue: https://support.microsoft.com/en-ph/help/4018140/computer-names-of-specialized-virtual-machines-are-missing-or-blank-in

Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>
Co-authored-by: haiyuan_zhang <haiyuan.zhang1982@gmail.com>

Add IPv6 address for azure_rm_subnet (ansible-collections#240)

* Add IPv6 address for azure_rm_subnet

Get address_prefixes info from virtualnetwork (ansible-collections#239)

* Add new parameter for get subnet IPv6 info

Support azure cli credentials with multiple `subscription_id`s (ansible-collections#195)

* feat: Support azure cli credentials with multiple `subscription_id`s

If the `subscription_id` is specified as module parameter or in the
environment then try to find that subscription in either the MSI (existing) or
CLI credentials (new). This patch brings those two scenarios in line.

* docs: Improve documentation on auth_source

* refactor: Move defaults up to class

Just trying to make the `__init__` fn a bit slimmer and easier to
reason about.

* refactor: Use python kwargs instead of passing dict

This is an isomorphic change, just using python syntax to accomplish
the exact same thing.

* refactor: Use ansible builtin `env_fallback` for `auth_source`

Ansible modules have a pattern for looking up a module parameter in
the environment with precedence of explicit param -> env -> default.
Use this pattern to simplify our code here.

This shouldn't change any behavior of `auth_source`, just using
standard ansible patterns to accomplish it.

* style: Split long line

No semantic change, just wrapping a long line to be a bit more
readable.

* refactor: helper fun _get_env

Somewhat frequently there is a lookup in the environment for the key that
matches a module parameter. This simple helper just encapsulates that
to make it a bit easier elsewhere-- lookup the same key in params,
credentials, env

* fix: typo in log message

Co-authored-by: Justin Ossevoort <justin.ossevoort@tesorion.nl>

Add 10.3 version for azure_rm_mariadbserver module (ansible-collections#244)

* add new version for azure_rm_mariadbserver

Update azure_rm_storageaccount yaml (ansible-collections#226)

* update azure_rm_storageaccount test
* update pipeline file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants