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

Fix diff_ignore_lines option issue for candidate configuration #45201

Merged
merged 2 commits into from Sep 6, 2018
Merged

Fix diff_ignore_lines option issue for candidate configuration #45201

merged 2 commits into from Sep 6, 2018

Conversation

ganeshrn
Copy link
Member

@ganeshrn ganeshrn commented Sep 5, 2018

SUMMARY
  • diff_ignore_lines option is to handle the running config fetch from
    remote host and ignore the lines that are auto-updated eg: commit time and date
  • This option should not be used while processing candidate (input) configuration
ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

{eos,ios,nxos,iosxr}_config

ANSIBLE VERSION
2.7
ADDITIONAL INFORMATION

@ganeshrn
Fix diff_ignore_lines option issue for candidate configuration
9c5a50f 
*  diff_ignore_lines option is to handle the running config fetch from
   remote host and ignore the lines that are auto updated eg: commit time and date
*  This option should not be used while processing candidate (input) configuration
@ansibot
Copy link
Contributor

ansibot commented Sep 5, 2018

cc @Qalthos @gdpak @justjais @mikewiebe @rahushen @rcarrillocruz @trishnaguha @tstoner
click here for bot help

@ansibot ansibot added affects_2.8 This issue/PR affects Ansible v2.8 bug This issue/PR relates to a bug. needs_triage Needs a first human triage before being processed. networking Network category nxos Cisco NXOS community support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Sep 5, 2018
@ganeshrn ganeshrn removed the needs_triage Needs a first human triage before being processed. label Sep 5, 2018
gdpak
gdpak reviewed Sep 5, 2018
View reviewed changes
Copy link
Contributor

@gdpak gdpak left a comment

Choose a reason for hiding this comment

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

Changes looks good. But we might need a good test case to cover this option. There seems to be one test case in test/integration/targets/ios_smoke/tests/cli/common_config.yaml but it does not check candidate config in result.updates. and probably configuring hostname might not be good config.

Can we modify it to catch this scenario ?

@ansibot
Copy link
Contributor

ansibot commented Sep 6, 2018

cc @privateip
click here for bot help

@ansibot ansibot added support:network This issue/PR relates to code supported by the Ansible Network Team. test This PR relates to tests. labels Sep 6, 2018
@ganeshrn
Copy link
Member Author

ganeshrn commented Sep 6, 2018 •
edited

@gdpak Added test cases in check mode for ios_config. In the first test case candidate and running config is the same hence updates key is not present in the result. In next test case, diff_ignore_lines is added to ignore one of the config lines, this line will be ignored from running-config but not from candidate config thus the updates key will be present in result with ignored config line thus ensuring diff_ignore_lines is not applied on candidate config.

@ganeshrn ganeshrn merged commit a3c137c into ansible:devel Sep 6, 2018
@ganeshrn ganeshrn deleted the handle_candidate_diff branch September 6, 2018 18:25
ganeshrn added a commit to ganeshrn/ansible that referenced this pull request Sep 6, 2018
@ganeshrn
Fix diff_ignore_lines option issue for candidate configuration (ansib…
afb7bbf 
…le#45201)

* Fix diff_ignore_lines option issue for candidate configuration

*  diff_ignore_lines option is to handle the running config fetch from
   remote host and ignore the lines that are auto updated eg: commit time and date
*  This option should not be used while processing candidate (input) configuration

* Fix review comment

(cherry picked from commit a3c137c)
@ganeshrn ganeshrn mentioned this pull request Sep 6, 2018
Merged
abadger pushed a commit that referenced this pull request Sep 6, 2018
@ganeshrn @abadger
Fix diff_ignore_lines option issue for candidate configuration (#45201)
d3225b0 
* Fix diff_ignore_lines option issue for candidate configuration

*  diff_ignore_lines option is to handle the running config fetch from
   remote host and ignore the lines that are auto updated eg: commit time and date
*  This option should not be used while processing candidate (input) configuration

* Fix review comment

(cherry picked from commit a3c137c)
@dagwieers dagwieers added iosxr Cisco IOSXR community cisco Cisco technologies labels Feb 22, 2019
@ansible ansible locked and limited conversation to collaborators Jul 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@gdpak gdpak gdpak left review comments

@trishnaguha trishnaguha trishnaguha approved these changes

@privateip privateip Awaiting requested review from privateip

@Qalthos Qalthos Awaiting requested review from Qalthos

Assignees
No one assigned
Labels
affects_2.8 This issue/PR affects Ansible v2.8 bug This issue/PR relates to a bug. cisco Cisco technologies iosxr Cisco IOSXR community networking Network category nxos Cisco NXOS community support:core This issue/PR relates to code supported by the Ansible Engineering Team. 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.

None yet
5 participants
@ganeshrn @ansibot @gdpak @trishnaguha @dagwieers