add source interface parameter to ios_logging

[lvrfrc87]

SUMMARY

Add arguments to support source interface for logging host

ISSUE TYPE

• Feature Pull Request

COMPONENT NAME

ios_logging

[ansibot]

cc @rcarrillocruz @trishnaguha
click here for bot help

[ansibot]

@FedericoOlivieri, just so you are aware we have a dedicated Working Group for network.
You can find other people interested in this in #ansible-network on Freenode IRC
For more information about communities, meetings and agendas see https://github.com/ansible/community

click here for bot help

[ansibot]

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/network/ios/ios_logging.py:0:0: E322 "source" is listed in the argument_spec, but not documented in the module

click here for bot help

[ansibot]

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/network/ios/ios_logging.py:0:0: E309 version_added for new option (source) should be 2.8. Currently 0.0

click here for bot help

[ansibot]

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/network/ios/ios_logging.py:0:0: E309 version_added for new option (source) should be 2.8. Currently 0.0

click here for bot help

[NilashishC]

@FedericoOlivieri Thanks a lot for this feature PR. The same option is defined as the interface parameter in the nxos_logging module and it would be great if the parameter names are consistent across all the platforms.
Also, could you please add some integration tests for this feature?

[lvrfrc87]

@NilashishC I have updated the code as required. I have noticed 2 things that they don't work as expected and I might need for you support because I cannot figure out what is wrong:

• Module is not Idempotent for source interface

changed: [uk-tempvpn8] => {
    "changed": true,
    "commands": [
        "logging source-interface FastEthernet0",
        "logging host 8.8.8.8"
    ],
    "invocation": {
        "module_args": {
            "aggregate": null,
            "auth_pass": null,
            "authorize": null,
            "dest": "host",
            "facility": null,
            "host": null,
            "level": "notifications",
            "name": "8.8.8.8",
            "password": null,
            "port": null,
            "provider": {
                "auth_pass": null,
                "authorize": null,
                "host": "10.108.49.161",
                "password": "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER",
                "port": null,
                "ssh_keyfile": null,
                "timeout": null,
                "username": "federico.olivieri"
            },
            "size": null,
            "source": "FastEthernet0",
            "ssh_keyfile": null,
            "state": "present",
            "timeout": null,
            "username": null
        }
    }
}

• Absent is not working as expected. That it happen for other parameters, not just for source interface. I see command list is always empty.

ok: [uk-tempvpn8] => {
    "changed": false,
    "commands": [],
    "invocation": {
        "module_args": {
            "aggregate": null,
            "auth_pass": null,
            "authorize": null,
            "dest": null,
            "facility": null,
            "host": null,
            "level": "notifications",
            "name": null,
            "password": null,
            "port": null,
            "provider": {
                "auth_pass": null,
                "authorize": null,
                "host": "10.108.49.161",
                "password": "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER",
                "port": null,
                "ssh_keyfile": null,
                "timeout": null,
                "username": "federico.olivieri"
            },
            "size": null,
            "source": null,
            "ssh_keyfile": null,
            "state": "absent",
            "timeout": null,
            "username": null
        }
    }
}

[NilashishC]

@FedericoOlivieri There appears to be a couple of different issues with the changes you made. Have a look at https://www.diffchecker.com/776O6EV0. I have made the required changes for this parameter to be idempotent.

Also, please note that configuring a logging interface should be a separate task and should not be combined with other parameters such as host in order for the module to function properly(i.e, be idempotent).

A brief example would be:

tasks:
    - name: Configure Interface
      ios_logging:
        interface: GigabitEthernet0/1
        state: present

    - name: Configure Host
      ios_logging:
        dest: host
        name: 8.8.8.8
        state: present

Edit: A comment below demonstrates the same with aggregate.

[lvrfrc87]

@NilashishC Thanks for your help. Just a question as I cannot figure out from the code: why the source interface has to be put in a separate task and not with the others?

[NilashishC]

@FedericoOlivieri The way the module currently works is - the map_config_to_obj function reads each line in the output of show running configuration | include logging and forms a dictionary for each line. So essentially, have and want both are list of dictionaries.

So for the following output of show running configuration | include logging:

logging source-interface GigabitEthernet0/1
logging host 8.8.8.8

have = [{'dest': None, 'facility': None, 'interface': u'GigabitEthernet0/1', 'level': 'debugging', 'name': None, 'size': None}, {'dest': u'host', 'facility': None, 'interface': None, 'level': 'debugging', 'name': u'8.8.8.8', 'size': None}]

If we specify host and interface configurations in the same task(without using aggregate), want would look like:

want=[{'dest': 'host', 'facility': None, 'interface': 'GigabitEthernet0/1', 'level': 'debugging', 'name': '8.8.8.8', 'size': None, 'state': 'present'}]

In essence, that would be a single dictionary with both the configurations.

Evidently, in this scenario, idempotency would not be preserved.

A better approach to this would be using aggregate:

tasks:
  ios_logging:
    aggregate:
      - { dest: host, name: 8.8.8.8, state: present }
      - { interface: GigabitEthernet0/1, state: present }

[lvrfrc87]

Thanks for the explanation. :)

[NilashishC]

@FedericoOlivieri It would be great if you could make the suggested changes so that we can go ahead and merge this PR. Thanks!

[lvrfrc87]

@NilashishC Which changes are we talking about? I understand you amended the code :) Is that right?

[NilashishC]

@FedericoOlivieri Actually, I did not amend the code. Based on your work, I pointed to the places which needs changes. A commit including those changes and some tests would be great!

[abenokraitis]

@FedericoOlivieri Greetings, just checking in on this - are you still supporting this PR? If so please let us know, else we will close this. Thanks!

[ansibot]

@FedericoOlivieri This pullrequest is waiting for your response. Please respond or the pullrequest will be closed.

click here for bot help