Add integration tests for nxos_facts, nxos_bgp, nxos_bgp_af, nxos_bgp_neighbor, and nxos_evpn_global #26924

rahushen · 2017-07-17T20:51:32Z

SUMMARY

This PR adds integration tests for the nxos_bgp, nxos_bgp_af, nxos_bgp_neighbor and nxos_evpn_global modules.

ISSUE TYPE

Integration Test Pull Request

COMPONENT NAME

nxos_bgp
nxos_bgp_af
nxos_bgp_neighbor
nxos_evpn_global

ANSIBLE VERSION

ansible 2.4.0 (devel 24195c9932) last updated 2017/07/17 16:45:40 (GMT -400)
  config file = None
  configured module search path = [u'/Users/rahushen/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /Users/xxx/xxxx/ansible/lib/ansible
  executable location = /Users/xxxx/xxxx/ansible/bin/ansible
  python version = 2.7.12 (v2.7.12:d33e0cf91556, Jun 26 2016, 12:10:39) [GCC 4.2.1 (Apple Inc. build 5666) (dot 3)]

ansibot · 2017-07-17T21:03:28Z

The test ansible-test sanity --test yamllint failed with the following errors:

test/integration/targets/nxos_bgp/tests/cli/sanity.yaml:77:7: duplication of key "router_id" in mapping (key-duplicates)
test/integration/targets/nxos_bgp/tests/nxapi/sanity.yaml:77:7: duplication of key "router_id" in mapping (key-duplicates)

click here for bot help

jctanner · 2017-07-19T12:21:36Z

cc @GGabriele @jedelman8 @mikewiebe @privateip @rahushen @rcarrillocruz @trishnaguha

ansibot · 2017-07-20T20:40:59Z

@rahushen this PR contains the following merge commits:

ec70c46

Please rebase your branch to remove these commits.

click here for bot help

Qalthos · 2017-07-26T19:02:20Z

Status of the tests provided:

nxos_bgp works after nxos_bgp: Fix idempotence for event-history #27329, but both neighbor-down fib-accelerate and reconnect-interval report as invalid commands
nxos_bgp_af works, but only after appending ignore_errors: yes to Disable feature nv overlay (which matches the Enable feature nv overlay task
nxos_bgp_neighbor is better after nxos_bgp_neighbor fixes #27348, with the caveats that shutdown no longer takes default as an option (and the corresponding line in sanity.yaml should be removed), that log-neighbor-changes kicks the commands out of bgp-neighbor into just bgp context, and remove private-as all complains with Please configure a public local-as with no-prepend replace-as to ensure that the real private ASN does not get appended to the AS-PATH
nxos_evpn_global looks good to me.

rahushen · 2017-08-02T15:13:37Z

@Qalthos

Fixed nxos_bgp_af.
nxos_bgp_neighbor - removed the default shutdown from test as it is not supported. And the other 2 issues are what is reference in (P2)nxos_bgp_neighbor - several properties not idempotent #26262. If log_neighbor_changes kicks the command into nxos_bgp then the property doesn't belong to this module ? If remove private-as all complains shouldn't the module fail with that message ? Instead all we get is idempotency check failures.

rahushen · 2017-08-03T18:59:06Z

@Qalthos .... I've added checks in the tests for titanium now. Can you verify ?

Qalthos

This works for me. There's some issues left to be resolved, but with those out of the way, we can finally merge this.

Qalthos · 2017-08-07T21:48:23Z

test/integration/targets/nxos_bgp/tests/cli/sanity.yaml

+      graceful_restart_timers_restart: 130
+      graceful_restart_timers_stalepath_time: 310
+      isolate: false
+      log_neighbor_changes: true


log_neighbor_changes needs to be toggled on titanium

Hahaha, it was totally my fault x_x

Sorry.

Qalthos · 2017-08-07T21:49:09Z

test/integration/targets/nxos_bgp/tests/nxapi/sanity.yaml

+      graceful_restart_timers_restart: 130
+      graceful_restart_timers_stalepath_time: 310
+      isolate: false
+      log_neighbor_changes: true


And the various parameters

Qalthos · 2017-08-07T22:28:14Z

test/integration/targets/nxos_bgp/tests/cli/sanity.yaml

+      log_neighbor_changes: true
+      maxas_limit: 50
+      neighbor_down_fib_accelerate: true
+      reconnect_interval: 55


And reconnect_interval

Qalthos · 2017-08-07T22:28:43Z

test/integration/targets/nxos_bgp/tests/cli/sanity.yaml

+      event_history_detail: size_large
+      event_history_events: size_medium
+      event_history_periodic: size_small
+      suppress_fib_pending: "{{suppress_fib_pending|default(omit)}}"


But I don't think suppress_fib_pending does

Qalthos · 2017-08-07T22:35:37Z

test/integration/targets/nxos_bgp/tests/cli/sanity.yaml

+- debug: msg="START TRANSPORT:CLI nxos_bgp sanity test"
+
+- set_fact: suppress_fib_pending="true"
+  when: (titanium is defined) and ((titanium | search('true')) != 'true')


This test did not work properly as written. when: (titanium is defined) and not ((titanium | search('true'))) did.

Qalthos · 2017-08-07T22:36:05Z

test/integration/targets/nxos_bgp/tests/nxapi/sanity.yaml

+- debug: msg="START TRANSPORT:NXAPI nxos_bgp sanity test"
+
+- set_fact: suppress_fib_pending="true"
+  when: (titanium is defined) and ((titanium | search('true')) != 'true')


Again with the test

Qalthos · 2017-08-07T22:37:08Z

test/integration/targets/nxos_bgp_neighbor/tests/cli/sanity.yaml

+- set_fact: intname="{{ nxos_int1 }}"
+
+- set_fact: log_neighbor_changes="enable"
+  when: (titanium is defined) and ((titanium | search('true')) != 'true')


Same issue with these tests

Qalthos · 2017-08-07T22:37:25Z

test/integration/targets/nxos_bgp_neighbor/tests/nxapi/sanity.yaml

+- set_fact: intname="{{ nxos_int1 }}"
+
+- set_fact: log_neighbor_changes="enable"
+  when: (titanium is defined) and ((titanium | search('true')) != 'true')


And these tests

rahushen · 2017-08-08T17:01:20Z

@Qalthos ... review comments have been take care of.

Qalthos · 2017-08-09T17:01:23Z

test/integration/targets/nxos_bgp/tests/cli/sanity.yaml

+---
+- debug: msg="START TRANSPORT:CLI nxos_bgp sanity test"
+
+- set_fact: log_neighbor_changes="true"


The comments I made here are gone, and I'm going to assume that this was my fault, because I've been looking at this too long and can't remember what is what anymore.

log_neighbor_changes is not an issue with nxos_bgp (but it is with nxos_bgp_neighbor). Instead, what needs to be turned off here in nxos_bgp (and its twin in nxapi) is neighbor_down_fib_accelerate. Sorry for the confusion

Ok fixed. If there are more issues with titanium specifically can we get a whole rundown on the issue rather than piece meal info ? This is incredibly frustating. Ideally, a test against an irrelevant platform shouldn't gate commits.

…pn_global

Qalthos · 2017-08-09T19:49:55Z

Everything passes here (after #27329 is applied). When Shippable is green, this is good to go

ansibot added affects_2.4 This issue/PR affects Ansible v2.4 needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Jul 17, 2017

jctanner added the networking Network category label Jul 19, 2017

jctanner added test_pull_requests and removed needs_triage Needs a first human triage before being processed. labels Jul 19, 2017

rahushen changed the title ~~Add new ITs for nxos_bgp, nxos_bgp_af, nxos_bgp_neighbor, and nxos_ev…~~ Add new ITs for nxos_facts, nxos_bgp, nxos_bgp_af, nxos_bgp_neighbor, and nxos_evpn_global Jul 20, 2017

ansibot added merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html labels Jul 20, 2017

rahushen force-pushed the devel branch from 7da2965 to e18fb4c Compare July 20, 2017 20:43

rahushen mentioned this pull request Jul 26, 2017

Network Working Group Meeting Agenda ansible/community#110

Closed

rahushen force-pushed the devel branch from 89a3d59 to 30c264d Compare July 26, 2017 15:37

ansibot removed needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jul 26, 2017

gundalow changed the title ~~Add new ITs for nxos_facts, nxos_bgp, nxos_bgp_af, nxos_bgp_neighbor, and nxos_evpn_global~~ Add integration tests for nxos_facts, nxos_bgp, nxos_bgp_af, nxos_bgp_neighbor, and nxos_evpn_global Jul 26, 2017

gundalow added this to the 2.4.0 milestone Jul 26, 2017

gundalow requested a review from Qalthos July 26, 2017 16:14

gundalow assigned Qalthos Jul 26, 2017

ansibot removed needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Aug 2, 2017

rahushen force-pushed the devel branch from 5b17bfc to 37a7997 Compare August 3, 2017 18:57

rahushen force-pushed the devel branch from 29d5a78 to b39c621 Compare August 7, 2017 19:05

Qalthos suggested changes Aug 7, 2017

View reviewed changes

ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Aug 7, 2017

rahushen force-pushed the devel branch from 3a26a2a to 018e793 Compare August 8, 2017 17:00

Qalthos suggested changes Aug 9, 2017

View reviewed changes

rahushen added 10 commits August 9, 2017 14:00

Add new ITs for nxos_bgp, nxos_bgp_af, nxos_bgp_neighbor, and nxos_ev…

1576eef

…pn_global

removing duplicated router_id from tests

c216f5f

Add nxos_facts IT and enhance existing tests

fa7b5f0

switch nxos_feature tests to use bgp

332fb15

misc. test fixes

c14c632

Add checks for titanium in IT

0d5e085

Handle non-titanium case in tests

cda6e1a

remove duplicate router_ids again

9930654

incorporate qalthos review comments

21309ad

addressing more review comments

ed7105a

rahushen force-pushed the devel branch from b8c9cb6 to ed7105a Compare August 9, 2017 18:00

ansibot added needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. and removed needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. labels Aug 9, 2017

Qalthos merged commit 85fc4c6 into ansible:devel Aug 10, 2017

dagwieers added nxos Cisco NXOS community cisco Cisco technologies labels Feb 22, 2019

ansible locked and limited conversation to collaborators Apr 26, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add integration tests for nxos_facts, nxos_bgp, nxos_bgp_af, nxos_bgp_neighbor, and nxos_evpn_global #26924

Add integration tests for nxos_facts, nxos_bgp, nxos_bgp_af, nxos_bgp_neighbor, and nxos_evpn_global #26924

rahushen commented Jul 17, 2017 •

edited

ansibot commented Jul 17, 2017

jctanner commented Jul 19, 2017

ansibot commented Jul 20, 2017

Qalthos commented Jul 26, 2017 •

edited

rahushen commented Aug 2, 2017

rahushen commented Aug 3, 2017

Qalthos left a comment

Qalthos Aug 7, 2017

Qalthos Aug 9, 2017

Qalthos Aug 7, 2017

Qalthos Aug 7, 2017

Qalthos Aug 7, 2017

Qalthos Aug 7, 2017

Qalthos Aug 7, 2017

Qalthos Aug 7, 2017

Qalthos Aug 7, 2017

rahushen commented Aug 8, 2017

Qalthos Aug 9, 2017

rahushen Aug 9, 2017

Qalthos commented Aug 9, 2017

Add integration tests for nxos_facts, nxos_bgp, nxos_bgp_af, nxos_bgp_neighbor, and nxos_evpn_global #26924

Add integration tests for nxos_facts, nxos_bgp, nxos_bgp_af, nxos_bgp_neighbor, and nxos_evpn_global #26924

Conversation

rahushen commented Jul 17, 2017 • edited

SUMMARY

ISSUE TYPE

COMPONENT NAME

ANSIBLE VERSION

ansibot commented Jul 17, 2017

jctanner commented Jul 19, 2017

ansibot commented Jul 20, 2017

Qalthos commented Jul 26, 2017 • edited

rahushen commented Aug 2, 2017

rahushen commented Aug 3, 2017

Qalthos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rahushen commented Aug 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Qalthos commented Aug 9, 2017

rahushen commented Jul 17, 2017 •

edited

Qalthos commented Jul 26, 2017 •

edited