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

nxos_bgp_neighbor: add local_as attributes #57714

Open
wants to merge 3 commits into
base: devel
from

Conversation

Projects
None yet
4 participants
@jacobsnapp
Copy link

commented Jun 11, 2019

SUMMARY

Added functionality to utilise additional attributes with the local-as command in nxos_bgp_neighbor module.

local-as autonomous-system-number [ no-prepend | replace-as [ dual-as ]]

This functionality has been implemented through the following boolean variables (default: False)

  • local_as_no_prepend
  • local_as_replace_as
  • local_as_dual_as
ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

nxos_bgp_neighbor

ADDITIONAL INFORMATION

I came across a use case that required these addditional attributes to convert our NXOS infrastructure BGP configuration to IaC using core ansible modules.

Example pulled from integration test:

  - name: "Configure BGP Neighbor Local AS"
    nxos_bgp_neighbor: &configure_local_as
      asn: 65535
      neighbor: 192.0.2.3/32
      vrf: "{{ item }}"
      local_as: 65523
      local_as_no_prepend: True
      local_as_replace_as: True
      local_as_dual_as: False
      state: present
    with_items: "{{ vrfs }}"
    register: result

Apologies about #57703 and #57712, still figuring out these git mechanisms

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jun 11, 2019

The test ansible-test sanity --test validate-modules [explain] failed with 3 errors:

lib/ansible/modules/network/nxos/nxos_bgp_neighbor.py:0:0: E309 version_added for new option (local_as_dual_as) should be '2.9'. Currently StrictVersion ('0.0')
lib/ansible/modules/network/nxos/nxos_bgp_neighbor.py:0:0: E309 version_added for new option (local_as_no_prepend) should be '2.9'. Currently StrictVersion ('0.0')
lib/ansible/modules/network/nxos/nxos_bgp_neighbor.py:0:0: E309 version_added for new option (local_as_replace_as) should be '2.9'. Currently StrictVersion ('0.0')

click here for bot help

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jun 11, 2019

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

click here for bot help

commands.append(command)
# If local_as arg is NOT passed to module execution -- only remove if it existed originally
elif module.params['local_as'] is None and existing.get('local_as'):
command = 'no local-as {0}'.format(existing.get('local_as'))

This comment has been minimized.

Copy link
@chrisvanheuveln

chrisvanheuveln Jun 19, 2019

Contributor

@trishnaguha Trishna, I reviewed this code; LGTM except for this if block which removes optional (non-playbook) values from the device. My understanding from Mike is that (for existing modules) we should be merging playbook changes into existing device configs. What's your take on this?
Thanks

This comment has been minimized.

Copy link
@mikewiebe

mikewiebe Jun 19, 2019

Contributor

Agreed. I don't think we want the default behavior to remove existing configuration from the device. If we end up supporting overridden state in the future then this would be a supported workflow.

This comment has been minimized.

Copy link
@jacobsnapp

jacobsnapp Jun 19, 2019

Author

Thanks for the review! I removed this elif-block and modified the appropriate unit test to ensure that the default behaviour does not override existing configuration on the device.

@jacobsnapp jacobsnapp force-pushed the jacobsnapp:nxos_bgp_neighbor_local_as branch from 42711fc to bb5a695 Jun 19, 2019

@ansibot ansibot added the core_review label Jun 19, 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.