Multi-level config parsing returns wrong indent leading to "always changed"

SUMMARY

Changes to config nested more than one parent down (eg, interface / vrrp-group) shows as "always changed" even if config lines present, because the parsed config is returned with a fixed indent level of 1.

ISSUE TYPE

Bug Report

COMPONENT NAME

dellemc.os10.plugins.module_utils.network.os10.py

ANSIBLE VERSION

(ansible) [ewen@castle switch-setup]$ ls roles/dellos6/patches/
dellemc_os6_os6_config-patch-33.diff  dellemc_os6_os6_config-patch-33.txt
(ansible) [ewen@castle switch-setup]$ ansible --version
ansible 2.10.17
  config file = /home/ewen/switch-setup/etc/ansible.cfg
  configured module search path = ['/home/ewen/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/ewen/ansible/lib64/python3.6/site-packages/ansible
  executable location = /home/ewen/ansible/bin/ansible
  python version = 3.6.8 (default, Nov 16 2020, 16:55:22) [GCC 4.8.5 20150623 (Red Hat 4.8.5-44)]
(ansible) [ewen@castle switch-setup]$

COLLECTION VERSION

(ansible) [ewen@castle switch-setup]$ ansible-galaxy collection list dellemc.os10

# /home/ewen/ansible/lib64/python3.6/site-packages/ansible_collections
Collection   Version
------------ -------
dellemc.os10 1.2.0  

# /home/ewen/ansible/lib/python3.6/site-packages/ansible_collections
Collection   Version
------------ -------
dellemc.os10 1.2.0  
(ansible) [ewen@castle switch-setup]$

CONFIGURATION

(ansible) [ewen@castle switch-setup]$ ansible-config dump --only-changed
COLLECTIONS_PATHS(env: ANSIBLE_COLLECTIONS_PATHS) = ['/home/ewen/ansible/lib/pyt
DEFAULT_HOST_LIST(env: ANSIBLE_INVENTORY) = ['/home/ewen/switch-setup/hosts']
DEFAULT_STDOUT_CALLBACK(/home/ewen/switch-setup/etc/ansible.cfg) = defaultsummar
TRANSFORM_INVALID_GROUP_CHARS(/home/ewen/switch-setup/etc/ansible.cfg) = ignore
(ansible) [ewen@castle switch-setup]$

OS / ENVIRONMENT

Ansible host is CentOS 7, with Ansible 2.10 installed from PIP (last version supported by Python 3.6 in CentOS 7). Target switch is DellEMC S5248F-ON, running 10.5.4.0.98.

STEPS TO REPRODUCE

Given a layer 3 vlan interface with a VRRP group on it

new-s5248f# show running-configuration interface vlan 99
!
interface vlan99
 description Test-VLAN
 no shutdown
 ip vrf forwarding test-vrf
 ip address 10.5.5.2/29
 !
 vrrp-group 99
  virtual-address 10.5.5.1
new-s5248f#

and the setup step:

- name: "Dell OS10 | Layer 3 | Configure VRRP address {{vrrp_ipv4_gateway}} (on {{interface_name}})"
  os10_config:
    lines:
      - 'virtual-address {{(vrrp_ipv4_gateway.split("/"))[0]}}'
    parents:
      - "interface {{interface_name}}"
      - "vrrp-group {{vrrp_group_id}}"
    replace: line
    match: line
    update: "{{'check' if ansible_check_mode else 'merge'}}"
    config: "{{ansible_net_config}}"

EXPECTED RESULTS

On the second run of applying the configuration for vlan 99, I expect it to show as "ok", as it has already been configured.

ACTUAL RESULTS

Instead on every run of applying the configuration for vlan 99, the vrrp-group task step is shown as changed. Every run, forever. Even though the correct configuration is on the switch.

With some "q" debugging, I narrowed that down to the code that handles reconstituting the configuration section to compare:

https://github.com/ansible-collections/dellemc.os10/blob/d343550a5e7dc0bef2f4bae96274087329dbc28a/plugins/module_utils/network/os10.py#L128-L146

not properly handling configuration indented more than one level deep. In particular the configuration is returned as str values, and the code assumes after outputting a single line everything else should be idented by exactly 1:

https://github.com/ansible-collections/dellemc.os10/blob/d343550a5e7dc0bef2f4bae96274087329dbc28a/plugins/module_utils/network/os10.py#L143

When there is more than one parent (indicating more than one level of nesting) this is incorrect; there should be one level of identing per parent.

I was able to work around this with the following replacement for get_sublevel_config(), which properly counts the number of indents required by the parents:

(ansible) [ewen@castle ansible_collections]$ grep -A 999 "get_sublevel_config" dellemc/os10/plugins/module_utils/network/os10.py
def get_sublevel_config(running_config, module):
    contents = list()
    current_config_contents = list()
    running_config = NetworkConfig(contents=running_config, indent=1)
    obj = running_config.get_object(module.params['parents'])
    if obj:
        contents = obj.children

    # Handle parent nesting first, so we get the indent depth required
    #
    indent = 0
    for c in module.params['parents']:
        if isinstance(c, str):
            current_config_contents.append(c.rjust(len(c) + indent, ' '))
            indent = indent + 1

    # Handle the remainder of the output, at the correct indent depth
    for c in contents:
        if isinstance(c, str):
            current_config_contents.append(c.rjust(len(c) + indent, ' '))
        if isinstance(c, ConfigLine):
            current_config_contents.append(c.raw)

    sublevel_config = '\n'.join(current_config_contents)

    return sublevel_config
(ansible) [ewen@castle ansible_collections]$

Once the correct indent level was returned the already configured vlan99 vrrp-group configuration then returned ok as expected, both in "dry run" mode and in actual apply changes mode.

And if the vlan99 vrrp-group config was manually removed, it was correctly reapplied by Ansible, and then on the subsequent runs correctly showed as "ok". (Both with just virtual-address manually removed, and then on another test with the whole vrrp-group manually removed.)

FTR, I found that the parsed config extracted was always str (ie, without indent built in) for the virtual-address command that should have been nested two deep; possibly whoever wrote the code was assuming it would be ConfigLine, but that does not appear to always be the case.

ansible-collections / dellemc.os10