-
Notifications
You must be signed in to change notification settings - Fork 330
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
Add support for application gateway path-based routing #452
Add support for application gateway path-based routing #452
Conversation
@l3ender Thank you for your contribution. We will complete the review and push forward the merger as soon as possible! Thank you very much! |
@l3ender Lines 820,821,830,831,936,837,877, 878 and 879 continuation line over indented for visual indent! Thank you very much! |
@l3ender There is something wrong with the newly added parameter in idempotency, could you please help to check it again? Thank you very much! |
@Fred-sun I've corrected indentation and added a test case for idempotency. Can you please review? If you're seeing other issues with idempotency please share detail! Thank you! |
@l3ender I have tested it and there is still something wrong with the idempotency of the parameter. Could you please try again?Also, when this parameter is added, the creation resource takes too long to use. Can you do something to optimize this? Thank you very much! |
2281f46
to
8dfc8ed
Compare
@Fred-sun I have corrected the idempotency issues you identified--thank you! Regarding the time taken to create a resource with this new feature, I am seeing very similar timings comparing app gateways with routing of either type (
vs
I created 3 new resources of each type and added timing around the module's call to azure-mgmt-network. Results:
Please let me know if there is anything else. Thank you for your patience with me while I complete this effort! |
@l3ender I have run the test case, but there are some problems. Can you help execute the test cases(ests/integration/targets/azure_rm_appgateway/tasks/main.yml) and provide the results? . Thank you very much! |
@l3ender Could you please help to authorize me to update this PR? Thank you very much! |
@Fred-sun I've sent an invite so you have access to update PR. I'm still trying to figure out how to run test cases, can you please advise? |
@l3ender You can try as following playbook ( include tests/integration/targets/azure_rm_appgateway/tasks/main.yml), Also copy the file under “tests/integration/targets/azure_rm_appgateway/files/cert*.txt” to the current path.
|
@Fred-sun I have run the test using your playbook. Here is the recap: PLAY RECAP PLAY RECAP *******************************************************************************************************
localhost : ok=26 changed=10 unreachable=0 failed=0 skipped=0 rescued=0 ignored=0 And here is the full execution output log: test-results2.txt. Thanks! |
Hi @Fred-sun, any update on this PR? |
…ctions#452) * support url path map for application gateway * update documentation and add test coverage * correct indentation * test idempotency for app gw with path based rules * correct merge * correct idempotency for path_based_routing * deep clone old/new props for accurate idempotency check * simplify path_based_routing examples and tests * prevent original parameter modification Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>
SUMMARY
This PR adds support for using path-based routing rules in an application gateway. This allows the request to be routed to different backend pools based on the URL path.
Fixes #448.
ISSUE TYPE
COMPONENT NAME
plugins/modules/azure_rm_appgateway.py
ADDITIONAL INFORMATION
This adds support for using
rule_type: path_based_routing
in a request routing rule. The routing rule references the URL path map resource for the gateway.