Repository: ansible/ansible
Pull request: Handling of configurations blocks with end-* at the end of the block
##### SUMMARY
Fixes #39351 

Few commands like route-policy and prefix-set have end-policy/end-set at the end of the block
e.g.
prefix-set ebpg_filter
  192.168.0.0/16 ge 17 le 30
end-set  

here current parser logic sets parent for line '192.168.0.0/16 ge 17 le 30' as 'prefix-set ebpg_filter'. It
also marks 'end-set' as new parent with no children.

Now say when we have a diff between candidate and running-config. we try to just play above config block without 'end-set' . This will cause all configs to fail below this line as command prompts will not accept any command until we come-out of this prefix-set context.

Solutions approach:
1. while parsing config set a new field 'trailing_parent' in config for such config blocks when we find new parent with "end-*'
2. Included end-* in diff so that it does not get excluded while comparing with running_config

1 approach is clean but much more intrusive in common code of parsing. Right now I have implemented and tested approach 2) we can discuss if there is alternative better approach

##### ISSUE TYPE
 - Bugfix Pull Request


##### COMPONENT NAME

lib/ansible/module_utils/network/common/config.py

##### ANSIBLE VERSION
<!--- Paste verbatim output from "ansible --version" between quotes below -->
```

```
2.6

Problem statement:
iosxr_config crash on NCS5508 6.2.3
<!---
Verify first that your issue/request is not already reported on GitHub.
THIS FORM WILL BE READ BY A MACHINE, COMPLETE ALL SECTIONS AS DESCRIBED.
Also test if the latest release, and devel branch are affected too.
ALWAYS add information AFTER (OUTSIDE) these html comments.
Otherwise it may end up being automatically closed by our bot. -->

##### ISSUE TYPE
<!--- Pick one below and delete the rest -->
 - Bug Report

##### COMPONENT NAME
<!--- Insert, BELOW THIS COMMENT, the name of the module, plugin, task or feature.
Do not include extra details here, e.g. "vyos_command" not "the network module vyos_command" or the full path-->
iosxr_config

##### ANSIBLE VERSION
<!--- Paste, BELOW THIS COMMENT, verbatim output from "ansible --version" between quotes below -->
```
ansible 2.5.0
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/Users/wblack/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/local/Cellar/ansible/2.5.0/libexec/lib/python2.7/site-packages/ansible
  executable location = /usr/local/bin/ansible
  python version = 2.7.14 (default, Apr  6 2018, 13:28:11) [GCC 4.2.1 Compatible Apple LLVM 9.1.0 (clang-902.0.39.1)]
```

##### CONFIGURATION
<!--- If using Ansible 2.4 or above, paste, BELOW THIS COMMENT, the results of "ansible-config dump --only-changed"
Otherwise, mention any settings you have changed/added/removed in ansible.cfg
(or using the ANSIBLE_* environment variables).-->
DEFAULT_JINJA2_EXTENSIONS(/etc/ansible/ansible.cfg) = jinja2.ext.do
DEFAULT_LOAD_CALLBACK_PLUGINS(/etc/ansible/ansible.cfg) = True
DEFAULT_STDOUT_CALLBACK(/etc/ansible/ansible.cfg) = debug
HOST_KEY_CHECKING(/etc/ansible/ansible.cfg) = False

##### OS / ENVIRONMENT
<!--- Mention, BELOW THIS COMMENT, the OS you are running Ansible from, and the OS you are
managing, or say "N/A" for anything that is not platform-specific.
Also mention the specific version of what you are trying to control,
e.g. if this is a network bug the version of firmware on the network device.-->
Ran From: Mac OS 10.13.4
Managing: Cisco NCS5508 with version 6.2.3

##### SUMMARY
<!--- Explain the problem briefly -->
Trying to push Jinja2 Template to NCS5508 running 6.2.3 caused Ansible to crash

##### STEPS TO REPRODUCE
<!--- For bugs, show exactly how to reproduce the problem, using a minimal test-case.
For new features, show how the feature would be used. -->
Use Ansible to generate router configuration using JINJA2.  Using that router configuration, push to device using iosxr_config.

<!--- Paste example playbooks or commands between quotes below -->
```yaml
- name: Starting IOS-XR NCS5500 AGG config push process
  hosts: svl-agg8
  gather_facts: no
  connection: network_cli

  roles:
    - xr-agg-ncs55k-config-push

- name: Push NCS55K AGG configs to {{ inventory_hostname }}
  iosxr_config:
    src: "{{ build_dir}}/{{ inventory_hostname }}.conf"
  check_mode: yes
```

<!--- You can also paste gist.github.com links for larger files -->

##### EXPECTED RESULTS
<!--- What did you expect to happen when running the steps above? -->
Should have received output for what configurations would be applied to router

##### ACTUAL RESULTS
<!--- What actually happened? If possible run with extra verbosity (-vvvv) -->

<!--- Paste verbatim command output between quotes below -->
```
dynd097:multi_roles wblack$ ansible-playbook -i hosts xr-agg-ncs55k-config-push.yml --diff --check --user <Redacted> -k -vvv
ansible-playbook 2.5.0
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/Users/wblack/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/local/Cellar/ansible/2.5.0/libexec/lib/python2.7/site-packages/ansible
  executable location = /usr/local/bin/ansible-playbook
  python version = 2.7.14 (default, Apr  6 2018, 13:28:11) [GCC 4.2.1 Compatible Apple LLVM 9.1.0 (clang-902.0.39.1)]
Using /etc/ansible/ansible.cfg as config file
SSH password: 
Parsed /Users/wblack/Desktop/Development/automated-config/ansible/multi_roles/hosts inventory source with ini plugin

PLAYBOOK: xr-agg-ncs55k-config-push.yml ***************************************************************************************************************************************************
1 plays in xr-agg-ncs55k-config-push.yml

PLAY [Starting IOS-XR NCS5500 AGG config push process] ************************************************************************************************************************************
META: ran handlers

TASK [xr-agg-ncs55k-config-push : Push NCS55K AGG configs to svl-agg8] ********************************************************************************************************************
task path: /Users/wblack/Desktop/Development/automated-config/ansible/multi_roles/roles/xr-agg-ncs55k-config-push/tasks/main.yml:2
Using module file /usr/local/Cellar/ansible/2.5.0/libexec/lib/python2.7/site-packages/ansible/modules/network/iosxr/iosxr_config.py
<127.0.0.1> ESTABLISH LOCAL CONNECTION FOR USER: wblack
<127.0.0.1> EXEC /bin/sh -c 'echo ~ && sleep 0'
<127.0.0.1> EXEC /bin/sh -c '( umask 77 && mkdir -p "` echo /Users/wblack/.ansible/tmp/ansible-tmp-1524722578.51-48392908457111 `" && echo ansible-tmp-1524722578.51-48392908457111="` echo /Users/wblack/.ansible/tmp/ansible-tmp-1524722578.51-48392908457111 `" ) && sleep 0'
<127.0.0.1> PUT /Users/wblack/.ansible/tmp/ansible-local-50608I8IpYm/tmp5BtrRJ TO /Users/wblack/.ansible/tmp/ansible-tmp-1524722578.51-48392908457111/iosxr_config.py
<127.0.0.1> EXEC /bin/sh -c 'chmod u+x /Users/wblack/.ansible/tmp/ansible-tmp-1524722578.51-48392908457111/ /Users/wblack/.ansible/tmp/ansible-tmp-1524722578.51-48392908457111/iosxr_config.py && sleep 0'
<127.0.0.1> EXEC /bin/sh -c '/usr/bin/python /Users/wblack/.ansible/tmp/ansible-tmp-1524722578.51-48392908457111/iosxr_config.py && sleep 0'
<127.0.0.1> EXEC /bin/sh -c 'rm -f -r /Users/wblack/.ansible/tmp/ansible-tmp-1524722578.51-48392908457111/ > /dev/null 2>&1 && sleep 0'
The full traceback is:
Traceback (most recent call last):
  File "/var/folders/72/96cc119x5j720g_392ms8tz40000gn/T/ansible_MeNxHV/ansible_module_iosxr_config.py", line 343, in <module>
    main()
  File "/var/folders/72/96cc119x5j720g_392ms8tz40000gn/T/ansible_MeNxHV/ansible_module_iosxr_config.py", line 337, in main
    run(module, result)
  File "/var/folders/72/96cc119x5j720g_392ms8tz40000gn/T/ansible_MeNxHV/ansible_module_iosxr_config.py", line 245, in run
    running_config = get_running_config(module)
  File "/var/folders/72/96cc119x5j720g_392ms8tz40000gn/T/ansible_MeNxHV/ansible_module_iosxr_config.py", line 221, in get_running_config
    contents = get_config(module)
  File "/var/folders/72/96cc119x5j720g_392ms8tz40000gn/T/ansible_MeNxHV/ansible_modlib.zip/ansible/module_utils/network/iosxr/iosxr.py", line 370, in get_config
  File "/var/folders/72/96cc119x5j720g_392ms8tz40000gn/T/ansible_MeNxHV/ansible_modlib.zip/ansible/module_utils/network/iosxr/iosxr.py", line 113, in get_connection
  File "/var/folders/72/96cc119x5j720g_392ms8tz40000gn/T/ansible_MeNxHV/ansible_modlib.zip/ansible/module_utils/network/iosxr/iosxr.py", line 129, in get_device_capabilities
  File "/var/folders/72/96cc119x5j720g_392ms8tz40000gn/T/ansible_MeNxHV/ansible_modlib.zip/ansible/module_utils/connection.py", line 146, in __rpc__
ansible.module_utils.connection.ConnectionError: show version brief
                                    ^
% Invalid input detected at '^' marker.
RP/0/RP0/CPU0:svl-agg8#

fatal: [svl-agg8]: FAILED! => {
    "changed": false, 
    "rc": 1
}

MSG:

MODULE FAILURE


MODULE_STDERR:

Traceback (most recent call last):
  File "/var/folders/72/96cc119x5j720g_392ms8tz40000gn/T/ansible_MeNxHV/ansible_module_iosxr_config.py", line 343, in <module>
    main()
  File "/var/folders/72/96cc119x5j720g_392ms8tz40000gn/T/ansible_MeNxHV/ansible_module_iosxr_config.py", line 337, in main
    run(module, result)
  File "/var/folders/72/96cc119x5j720g_392ms8tz40000gn/T/ansible_MeNxHV/ansible_module_iosxr_config.py", line 245, in run
    running_config = get_running_config(module)
  File "/var/folders/72/96cc119x5j720g_392ms8tz40000gn/T/ansible_MeNxHV/ansible_module_iosxr_config.py", line 221, in get_running_config
    contents = get_config(module)
  File "/var/folders/72/96cc119x5j720g_392ms8tz40000gn/T/ansible_MeNxHV/ansible_modlib.zip/ansible/module_utils/network/iosxr/iosxr.py", line 370, in get_config
  File "/var/folders/72/96cc119x5j720g_392ms8tz40000gn/T/ansible_MeNxHV/ansible_modlib.zip/ansible/module_utils/network/iosxr/iosxr.py", line 113, in get_connection
  File "/var/folders/72/96cc119x5j720g_392ms8tz40000gn/T/ansible_MeNxHV/ansible_modlib.zip/ansible/module_utils/network/iosxr/iosxr.py", line 129, in get_device_capabilities
  File "/var/folders/72/96cc119x5j720g_392ms8tz40000gn/T/ansible_MeNxHV/ansible_modlib.zip/ansible/module_utils/connection.py", line 146, in __rpc__
ansible.module_utils.connection.ConnectionError: show version brief
                                    ^
% Invalid input detected at '^' marker.
RP/0/RP0/CPU0:svl-agg8#


	to retry, use: --limit @/Users/wblack/Desktop/Development/automated-config/ansible/multi_roles/xr-agg-ncs55k-config-push.retry

PLAY RECAP ********************************************************************************************************************************************************************************
svl-agg8                   : ok=0    changed=0    unreachable=0    failed=1   
```

Hints from issue discussion:
Files identified in the description:
* [lib/ansible/modules/network/iosxr/iosxr_config.py](https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/network/iosxr/iosxr_config.py)

If these files are inaccurate, please update the `component name` section of the description or use the `!component` bot command.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->
cc @gdpak @privateip @rcarrillocruz
[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: notify --->
@eclecticitguy Earlier it was reported by just virtual XR. Looks like cisco has changed this command in  6.3.2 release for all XR products. It is fixed by https://github.com/ansible/ansible/pull/37609 . Can you try this fix ?
I do not see a fix in https://github.com/ansible/ansible/pull/37609 that
applies to my issue.  The latest comment in
https://github.com/ansible/ansible/pull/37609 from @mirochon indicates that
the problem not only isn't fixed but is worse starting in IOS-XR version
6.3.2.

If I missing something, please let me know.

Thanks.

On Thu, Apr 26, 2018 at 12:06 AM, Deepak Agrawal <notifications@github.com>
wrote:

> @eclecticitguy <https://github.com/eclecticitguy> Earlier it was reported
> by just virtual XR. Looks like cisco has changed this command in 6.3.2
> release for all XR products. It is fixed by #37609
> <https://github.com/ansible/ansible/pull/37609> . Can you try this fix ?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/ansible/ansible/issues/39351#issuecomment-384535555>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AL_3716fm-xKt2t1YuuR76VdkW9GJd9eks5tsXHegaJpZM4Tklli>
> .
>

fix in #37609 does not only apply to iosxr_facts but to iosxr_config also as while getting device capabilites in setting up connection to iosxr we were expecting 'show version brief' to be working. 
@gdpak  I pulled the latest Ansible development release and retried my configuration.  The previous "show version brief" error is no longer there but I'm still getting an error:

```
fatal: [svl-agg8]: FAILED! => {
    "changed": false, 
    "rc": 1
}

MSG:

MODULE FAILURE


MODULE_STDERR:

Traceback (most recent call last):
  File "/var/folders/72/96cc119x5j720g_392ms8tz40000gn/T/ansible_NJJ1Gq/ansible_module_iosxr_config.py", line 336, in <module>
    main()
  File "/var/folders/72/96cc119x5j720g_392ms8tz40000gn/T/ansible_NJJ1Gq/ansible_module_iosxr_config.py", line 330, in main
    run(module, result)
  File "/var/folders/72/96cc119x5j720g_392ms8tz40000gn/T/ansible_NJJ1Gq/ansible_module_iosxr_config.py", line 272, in run
    diff = load_config(module, commands, commit=commit, replace=replace_config, comment=comment, admin=admin)
  File "/var/folders/72/96cc119x5j720g_392ms8tz40000gn/T/ansible_NJJ1Gq/ansible_modlib.zip/ansible/module_utils/network/iosxr/iosxr.py", line 414, in load_config
  File "/var/folders/72/96cc119x5j720g_392ms8tz40000gn/T/ansible_NJJ1Gq/ansible_modlib.zip/ansible/module_utils/connection.py", line 149, in __rpc__
ansible.module_utils.connection.ConnectionError: =================================================================================
                               ^
% Invalid input detected at '^' marker.
RP/0/RP0/CPU0:svl-agg8(config)#
``` 
@eclecticitguy This error is coming from IOS-XR .As you have upgraded your iosxr version , probably some of your configs in config file are not compatible with new ios-xr. Please check your ios-xr config file given in 'src' argument to iosxr_config module for possible in-compatibility with ios-xr.
I found the cause of this issue.  It's not an incompatible configuration but rather a prefix-set that's already been configured and IOS-XR is throwing a warning about overwriting the prefix-set:

`% WARNING: Policy object prefix-set Conditional_Routes' exists! Reconfiguring it via CLI will replace current definition. Use 'abort to cancel.`

Is there any logic within the iosxr_config module that deals with prefix-sets that already exist?  If not, is there a possibility of adding this logic so that the module doesn't crash during execution?
@eclecticitguy iosxr_config would not try to change existing prefix-set (or any other config for that matter) if config present in system are same as provided in 'src' config file. so there is something different inside prefix-set you are trying to program vs what already exists on device.

from error log it looks like 'commit' was tried when command prompt was in prefix subcommand mode
RP/0/RP0/CPU0:ios(config-pfx)#commit
                              ^
% Invalid input detected at '^' marker

Can you check your config file has "end-set" line after configuring prefix-set. It should be at same level as prefix level-set.
e.g.
prefix-set ebpg_filter
  192.168.0.0/16 ge 15 le 30
  end-set
  ^^^^ end-set at subcommand level (same level as prefix 192.168....) 


@gdpak After your last comment, I went through my configuration template and it appears that there are issues if you use leading whitespace in your template file that matches the syntax of a real IOS-XR configuration.  In your example, you indicate that the end-set and attribute level should be at the same level as the prefix set:

```
prefix-set ebpg_filter
192.168.0.0/16 ge 15 le 30
end-set
```

However, in the IOS-XR configuration itself, there is leading whitespace to denote the config element hierarchy:

```
prefix-set ebpg_filter
  192.168.0.0/16 ge 15 le 30
end-set
```

I can apply either of these two configurations directly to the router without any issues but if I use the second option in my template config, Ansible crashes.  Does this mean I can't have any leading whitespace in my templates at all, even though IOS-XR will accept the configuration with the whitespace?  If this is the case, it's going to be very difficult to maintain these template files if you can't use whitespace to separate the configuration elements as IOS-XR does when looking at the configuration with "show run".

Commit message: fix trailing parents not included in difference

Patch to review:
diff --git a/lib/ansible/module_utils/network/common/config.py b/lib/ansible/module_utils/network/common/config.py
index 23b4df6a85ff1b..09e912203496b7 100644
--- a/lib/ansible/module_utils/network/common/config.py
+++ b/lib/ansible/module_utils/network/common/config.py
@@ -290,6 +290,9 @@ def _diff_line(self, other):
         for item in self.items:
             if item not in other:
                 updates.append(item)
+            # special trailing parents need to be included in diff
+            elif item.line == 'end-policy' or item.line == 'end-set':
+                updates.append(item)
         return updates
 
     def _diff_strict(self, other):


Code review comments:
diff_hunk: @@ -290,6 +290,9 @@ def _diff_line(self, other):
         for item in self.items:
             if item not in other:
                 updates.append(item)
+            # special trailing parents need to be included in diff
+            elif item.line == 'end-policy' or item.line == 'end-set':
path: lib/ansible/module_utils/network/common/config.py
text: @user1:
It seems this configuration is specific for a particular platform. Is it possible to sanitize the config indentation in the module code?  

@author:
right now there is no platform specific instance of NetworkConfig. It can be done to inherit this class at iosxr code and overwrite function 'difference' and 'diff_exact' to handle these special commands. will that work ?

@author:
pros of above approach: no extra check penalty for other OS, no regression on other
con: code duplication and fixes in common code would need to be ported