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

VMware: Adding IP-Specific properties to ESXi host firewall config #56733

Open
wants to merge 7 commits into
base: devel
from

Conversation

Projects
None yet
3 participants
@alongchamps
Copy link

commented May 21, 2019

SUMMARY

Implements 51240 so that admins can configure firewall rules with IP and Network information.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

vmware_host_firewall_manager

ADDITIONAL INFORMATION

Adds IP-Specific properties to firewall config
Implements ansible ticket 51240 so that admins can configure firewall rules on a per-rule level with IP-specific information.
@ansibot

This comment has been minimized.

@ansibot

This comment has been minimized.

Copy link
Contributor

commented May 21, 2019

@alongchamps, just so you are aware we have a dedicated Working Group for vmware.
You can find other people interested in this in #ansible-vmware on Freenode IRC
For more information about communities, meetings and agendas see https://github.com/ansible/community

click here for bot help

alongchamps added some commits May 21, 2019

Adds IP-Specific properties to firewall config
Implements ansible ticket 51240 so that admins can configure firewall rules on a per-rule level with IP-specific information.
Update main.yml
Changes hostvars ESXi list to use esxi1, esxi2
Fixed indentation
Fixed indentation on main.yaml for tests and also removed unicode usage.

@alongchamps alongchamps reopened this May 21, 2019

Fix vmware_host_firewall_manager.py indentation
Fix vmware_host_firewall_manager.py indentation in RETURN block
@ansibot

This comment has been minimized.

Copy link
Contributor

commented May 21, 2019

@alongchamps this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@ansibot

This comment has been minimized.

Copy link
Contributor

commented May 22, 2019

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/cloud/vmware/vmware_host_firewall_manager.py:152:21: E313 RETURN is not valid YAML

The test ansible-test sanity --test yamllint [explain] failed with 1 error:

lib/ansible/modules/cloud/vmware/vmware_host_firewall_manager.py:152:21: error RETURN: syntax error: expected the node content, but found '}'

click here for bot help

@ansibot ansibot added the ci_verified label May 22, 2019

@goneri
Copy link
Contributor

left a comment

Hi @alongchamps, thanks for the contribution. It took the freedom to do a quick review even in the PR has the Draft flag. For the record, you branch needs a rebase.

Cleaning up
Cleaned up some leftover debugging statements and shortened some boolean decisions. Also hopefully fixed the example return object and fleshed out the returns examples.

@ansibot ansibot removed the ci_verified label May 22, 2019

@alongchamps

This comment has been minimized.

Copy link
Author

commented May 22, 2019

@goneri I did do a merge with the changes on the integration test file, main.yml. I think that would get rid of the need to rebase since it's clean now?

@goneri

This comment has been minimized.

Copy link
Contributor

commented May 22, 2019

Yes, both are find, I personally prefer to rebase since it keeps the patches of my branch on top of the others.

@alongchamps alongchamps marked this pull request as ready for review May 23, 2019

@Akasurde Akasurde changed the title Adding IP-Specific properties to ESXi host firewall config VMware: Adding IP-Specific properties to ESXi host firewall config May 28, 2019

@goneri

goneri approved these changes Jun 3, 2019

@ansibot ansibot added stale_ci and removed needs_revision labels Jun 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.