cli_config requires improvement

[wisotzky]

Hi @trishnaguha,

The current implementation of cli_config is insufficient. If device plugins do neither support onbox diff nor generate_diff the configuration changes would not be carried out to the device.

Proposed change: If neither onbox_diff nor generate_diff are supported connection.edit_config(**kwargs) should still be called. A configuration snapshot called running is already taken before the config change, while for this specific case we would take another configuration snapshot running2 after the change. If both snapshots are identical, set result['changed']=False. If snapshots are different, set result['changed']=True. Also update result['diff'] with the value {'before': running, 'after': running2}.

With this proposal, both change indicator and --diff mode work very reliable. It enables low cost implementation of Ansible CLI plugins, especially if the CLI engine does not support candidates and CLI structure is not well-formed so generate_diff would become very expensive.

Need to say, this approach comes typically at a cost:

In those scenarios typically dryRun (check-mode) is not supported.

Taking a config snapshot might take time, especially for super-big configurations.

However, as cli_config is implemented very lazy aka it takes one config snapshot anyways, looks to me that this was not seen as a problem before. Realistically we may double here the execution time of the cli_config module - but people gain visibility, which is likely valued higher.

I've done the code changes on my Ansible 2.9 bench within 20min... So guess it should be a no-brainer for you to make this change into the new netcommon implementation.

https://github.com/ansible-collections/ansible.netcommon/blob/abb8735c3bd1e08d48abebbb0c25bec60bf2f6f4/plugins/modules/cli_config.py#L257

[ganeshrn]

@webknjaz The functionality that you mentioned can be achieved using cli_command module within the playbook. For example:

- name: get config snaspot1
  cli_command:
    command: show running-configuration
  register: snapshot1

- name: push configuration
  cli_command:
    command: "{{ lookup('file', '<path to config file>') }}"

- name: get config snaspot2
  cli_command:
    command: show running-configuration
  register: snapshot2

- name: check diff
  set_fact:
    config_changed: true
  when: "'{{ snapshot1.stdout }}' != '{{ snapshot2.stdout }}'"

The cliconf_config expects the platform to either support on box diff or implement the diff logic within Ansible for it to be idempotent and support check mode.

[wisotzky]

@ganeshrn, it's not the same. The proposal I've made supports properly both the regular change indicator and the --diff option without the need to break this down into 4 individual calls. In your case differences are not displayed.

So the proposal is the following: Use the same principles as we've got with NETCONF today aka module netconf_config if the device plugin does not support any diff options. In this case check mode would not be supported, but again it's along the lines with NETCONF if a device does not support the candidate datastore.

Regarding idempotency the bar is much higher, as this is more than just diff and dryRun (check mode). Realistically this is addressed by the vendor-specific resource modules in Ansible - but using CLI syntax it is hard to express the desired state (intended state) and there are not many CLI engines that provide NETCONF-like replace and remove operations that could be claimed to behave idempotent.