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

Jump to bottom

Add disable_outbound_snat configuration to azure_rm_loadbalancer module #744

Merged
merged 2 commits into from
Jun 7, 2022
Merged

Add disable_outbound_snat configuration to azure_rm_loadbalancer module #744

merged 2 commits into from
Jun 7, 2022

Conversation

tfmark
Copy link
Contributor

@tfmark tfmark commented Jan 26, 2022

SUMMARY

Fixes #743

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

azure_rm_loadbalancer

ADDITIONAL INFORMATION
  - name: Add Load Balancer rules
    azure.azcollection.azure_rm_loadbalancer:
      profile: prod
      resource_group: dummy-resource-group
      name: dummy-load-balancer
      load_balancing_rules :
        - name: dummy-rule-1
          backend_port: 9999
          protocol: Tcp
          frontend_port: 9999
          frontend_ip_configuration: dummy-load-balancer-ip
          backend_address_pool : dummy-load-balancer-pool
          probe: dummy-health-probe
          disable_outbound_snat: true # << new feature

@Fred-sun Fred-sun added enhancement New feature or request medium_priority Medium priority new_feature New feature requirments work in In trying to solve, or in working with contributors labels Feb 12, 2022
@Fred-sun
Copy link
Collaborator

@tfmark Can you add a test case for this newly added parameter? Thank you very much!

Fred-sun
Fred-sun reviewed Mar 4, 2022
View reviewed changes
plugins/modules/azure_rm_loadbalancer.py Outdated
@@ -207,6 +207,12 @@
enable_floating_ip:
description:
- Configures a virtual machine's endpoint for the floating IP capability required to configure a SQL AlwaysOn Availability Group.
disable_outbound_snat:
description:
- Configure outbound source network address translation (SNAT)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- Configure outbound source network address translation (SNAT)
- Configure outbound source network address translation (SNAT).

plugins/modules/azure_rm_loadbalancer.py Outdated
disable_outbound_snat:
description:
- Configure outbound source network address translation (SNAT)
- The default value is False
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- The default value is False
- The default value is I(disable_outbound_snat=False).

plugins/modules/azure_rm_loadbalancer.py Outdated
description:
- Configure outbound source network address translation (SNAT)
- The default value is False
- True is equivalent to "Use outbound rules to provide backend pool members access to the internet" in portal
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- True is equivalent to "Use outbound rules to provide backend pool members access to the internet" in portal
- True is equivalent to "Use outbound rules to provide backend pool members access to the internet" in portal.

plugins/modules/azure_rm_loadbalancer.py Outdated
- Configure outbound source network address translation (SNAT)
- The default value is False
- True is equivalent to "Use outbound rules to provide backend pool members access to the internet" in portal
- False is equivalent to "Use implicit outbound rule" in portal
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- False is equivalent to "Use implicit outbound rule" in portal
- False is equivalent to "Use implicit outbound rule" in portal.

@Fred-sun
Copy link
Collaborator

@tfmark Are you still contributing to this PR? Please help rebase this PR and fix the problems in the comments above! Thank you very much!

@tfmark
Copy link
Contributor Author

tfmark commented Mar 31, 2022

Hi @Fred-sun, I will look into this, but this is something work-related rather than something I do for fun. Last time I made a PR I found that it was difficult to test, but maybe things have changed.

I'll get around to it at some point.

@Fred-sun
Copy link
Collaborator

Fred-sun commented Apr 1, 2022

@tfmark It is not recommended to set a default value for this parameter, because if the resource already exists and we create it again, if we do not set this parameter, it will be updated to the default value. But we can document that the default is true or false. Thank you very much!

@Fred-sun Fred-sun removed the work in In trying to solve, or in working with contributors label Apr 7, 2022
@tfmark tfmark force-pushed the azure-rm-loadbalancer-disable-outbound-snat branch from 8905770 to ec21b5f Compare April 12, 2022 07:56
@Fred-sun Fred-sun added the ready_for_review The PR has been modified and can be reviewed and merged label May 24, 2022
@xuzhang3
Copy link
Collaborator

xuzhang3 commented Jun 7, 2022

LGTM

@xuzhang3 xuzhang3 merged commit 3a5d792 into ansible-collections:dev Jun 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Fred-sun Fred-sun Fred-sun left review comments

Assignees

@xuzhang3 xuzhang3

Labels
enhancement New feature or request medium_priority Medium priority new_feature New feature requirments ready_for_review The PR has been modified and can be reviewed and merged
Projects
None yet
Milestone
No milestone
3 participants
@tfmark @Fred-sun @xuzhang3