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

To fix iosxr_l3_interfaces module zuul tests #61592

Merged
merged 2 commits into from
Aug 30, 2019
Merged

To fix iosxr_l3_interfaces module zuul tests #61592

merged 2 commits into from
Aug 30, 2019

Conversation

justjais
Copy link
Contributor

@justjais justjais commented Aug 30, 2019 •
edited
Loading

SUMMARY

PR fixes iosxr_l3_interfaces zuul tests failing intermittently for different python run, raised in issue #61540 and also, contain fix for slight issue observed in Override and Replace scenario #61600

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

iosxr_l3_interfaces

ADDITIONAL INFORMATION

@ansibot
Copy link
Contributor

ansibot commented Aug 30, 2019

cc @NilashishC @Qalthos @danielmellado @ganeshrn @trishnaguha
click here for bot help

@ansibot ansibot added affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. cisco Cisco technologies core_review In order to be merged, this PR must follow the core review workflow. ios Cisco IOS community iosxr Cisco IOSXR community needs_triage Needs a first human triage before being processed. networking Network category small_patch support:network This issue/PR relates to code supported by the Ansible Network Team. test This PR relates to tests. labels Aug 30, 2019
@ansible-zuul
Copy link

ansible-zuul bot commented Aug 30, 2019

Build failed (third-party-check pipeline) integration testing with
Ansible.

@justjais
Copy link
Contributor Author

recheck

danielmellado
danielmellado reviewed Aug 30, 2019
View reviewed changes
lib/ansible/module_utils/network/iosxr/utils/utils.py
@@ -96,6 +96,10 @@ def filter_dict_having_none_value(want, have):
if each.get('secondary') and diff_ip is True:
test_key_dict.update({'secondary': True})
test_dict.update({'ipv4': test_key_dict})
# Checks if want doesn't have secondary IP but have has secondary IP set
Copy link
Contributor

Choose a reason for hiding this comment

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

so the issue was related with this? I't be great to have some description about the fix around. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@danielmellado the issue wasn't around this code fix, the problem was with how iosxr handles virtual and logical interfaces, previously I was using virtual and logical interfaces combo in integration tests and now have moved the tests to use zuul physical interface only instead of virtual ones and this resulted in solving the intermittent failures. I came across for the above code fix when I was trying with certain use cases myself, and that's why the respective comment for the fix

Copy link
Contributor

Choose a reason for hiding this comment

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

@justjais great to know! would you mind updating the PR message to include a summary of this? I think it'd be easier afterwards to check what the root case/root fix was.

Besides that the code LGTM!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@danielmellado sure, will open an issue with the description and link to the PR. :)

@ansibot ansibot removed the needs_triage Needs a first human triage before being processed. label Aug 30, 2019
danielmellado
danielmellado approved these changes Aug 30, 2019
View reviewed changes
Copy link
Contributor

@danielmellado danielmellado left a comment

Choose a reason for hiding this comment

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

Waiting on Zuul + PR description update ;)

@ansible-zuul
Copy link

ansible-zuul bot commented Aug 30, 2019

Build failed (third-party-check pipeline) integration testing with
Ansible.

@pabelanger
Copy link
Contributor

recheck

@Qalthos Qalthos mentioned this pull request Aug 30, 2019
Merged
@cidrblock cidrblock mentioned this pull request Aug 30, 2019
Merged
@pabelanger
Copy link
Contributor

@justjais this doesn't fix the issue 100%, but is an improvement. See the following for a failure:

https://object-storage-ca-ymq-1.vexxhost.net/v1/a0b4156a37f9453eb4ec7db5422272df/ansible_47/60947/e404e5210034fd5c504f5ecca8987f2b19b50bf9/third-party-check/ansible-test-network-integration-iosxr-python35/e5479ce/controller/ara-report/

@ansible-zuul
Copy link

ansible-zuul bot commented Aug 30, 2019

Build succeeded (third-party-check pipeline).

@pabelanger
Copy link
Contributor

So, the error above is different then the original issue of iosxr_l3_interfaces : Assert that after dict is correctly generated failing. Given the impact of this failure to our testing, I'm inclined to merge this PR and work to fix above in another. I am just waiting for another PR to finish testing this for more test results.

@pabelanger
Copy link
Contributor

Okay, given this seems to fix the original issue, I am going to merge. But we also need to work on the error above.

@pabelanger pabelanger merged commit 24ad1df into ansible:devel Aug 30, 2019
adharshsrivatsr pushed a commit to adharshsrivatsr/ansible that referenced this pull request Sep 3, 2019
@justjais @adharshsrivatsr
To fix iosxr_l3_interfaces module zuul tests (ansible#61592)
ad25f27 
* fix 61540

* fix utils
pabelanger pushed a commit to pabelanger/ansible that referenced this pull request Sep 3, 2019
@justjais @pabelanger
To fix iosxr_l3_interfaces module zuul tests (ansible#61592)
aac18c9 
* fix 61540

* fix utils

(cherry picked from commit 24ad1df)
abadger pushed a commit that referenced this pull request Sep 4, 2019
@justjais @abadger
To fix iosxr_l3_interfaces module zuul tests (#61592)
8141aa9 
* fix 61540

* fix utils

(cherry picked from commit 24ad1df)
anas-shami pushed a commit to anas-shami/ansible that referenced this pull request Sep 23, 2019
@justjais @anas-shami
To fix iosxr_l3_interfaces module zuul tests (ansible#61592)
c2d1b6e 
* fix 61540

* fix utils
@ansible ansible locked and limited conversation to collaborators Oct 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@pabelanger pabelanger pabelanger approved these changes

@danielmellado danielmellado danielmellado approved these changes

Assignees
No one assigned
Labels
affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. cisco Cisco technologies core_review In order to be merged, this PR must follow the core review workflow. ios Cisco IOS community iosxr Cisco IOSXR community networking Network category small_patch support:network This issue/PR relates to code supported by the Ansible Network Team. test This PR relates to tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@justjais @ansibot @pabelanger @danielmellado