Does a connection plugin in ansible-core make sense (for napalm)

[ktbyers]

Some of the issues involved:

1. Should it be connection: napalm instead of connection: local
2. Support for -u username -k (we currently support this using an action plugin)
3. Support for no arguments (no provider; no first-level connection arguments)
4. How timeouts work
5. Long-term support for provider (I probably have a bias towards not removing provider in napalm-ansible for backwards compatibility and I am unsure how napalm-ansible's optional_args would work without this).
6. Long-term support for first-level arguments in napalm-ansible.
7. Support for {{ ansible_network_os }}

[ktbyers]

Related item:

https://github.com/ansible/ansible/pull/45224

[cs-1]

@ktbyers I think that the first argument is very compelling and makes perfect sense (connection: local was very counter intuitive when we startet using napalm-ansible in Ansible). In any case, in my eyes keeping the backward compatibility is a must.

[abenokraitis]

FYI, the NAPALM connection plugin is already merged into the Ansible 2.8 development tree per joint discussions back in August. The Ansible community would be glad help the NAPALM community refactor the NAPALM modules to take advantage of the plugin, but based on the direction of Ansible community and Ansible network architecture, it seems like the most sense to just slightly modify cli_command and cli_config to work with the NAPALM plugin instead of refactoring all the existing modules.

[ktbyers]

@abenokraitis I didn't follow your statement?

it seems like the most sense to just slightly modify cli_command and 
cli_config to work with the NAPALM plugin instead of refactoring all 
the existing modules.

Which existing modules are you referring to in the all the existing modules? Can you expand on what you mean here?

Note, I think all of the above except 1 and 4 work in napalm-ansible as it currently is.

[abenokraitis]

@ktbyers Sure thing - The way I see it there are two options to get NAPALM working with the connection plug-in: 1) Refactor the NAPALM modules (there are 8 or so of them, correct?) 2) Modify cli_config and cli_command already in Ansible Core and then no modifications to the NAPALM modules would required

[ktbyers]

There are eight modules, but they all use one action plugin for argument handling.

So one option is to do nothing (which I am not seeing much negative from an end-user perspective). The negatives I see:

1. Need to use connection: local instead of connection: napalm (which seems fine to me)
2. Timeout behavior might be different than timeout behavior for core modules. This possibly would be pretty easy to fix, however.
3. Requires installation of napalm-ansible action_plugin which I know @itdependsnetworks doesn't like, but I view as unimportant.

What are the implications of modifying cli_config and cli_command? How would this work? Meaning napalm functionality would be moved into those two entities and replace napalm-ansible modules?

[ktbyers]

Adding @mirceaulinic and @dbarrosop onto this. In case they are interested.

I also had pinged @abenokraitis on Slack about how napalm would even integrate to the connection plugin. In other words, how would napalm even use the connection created by the ansible-napalm connection plugin (as this was very unclear to me).

[ktbyers]

Here are the items that I saw that were broke in the napalm ansible connection plugin:

1. No optional args support
2. Broke handling of username/password

Note, broken but it was entirely unclear to me how I would actually use the connection created by the connection plugin. Currently what happens in the connection plugin creates a connection and then the napalm-ansible modules ignore this and create their own connection.

[Qalthos]

Here are the items that I saw that were broke in the napalm ansible connection plugin:

1. No optional args support

Yes, this was a thing that I wanted to revisit and then other things came up. With connection options, this shouldn't be too hard to add.

2. Broke handling of username/password

I don't understand this one, though that may be down to insufficient testing. Every device I tried connected properly. Could you be more specific in what exactly is not right?

Note, broken but it was entirely unclear to me how I would actually use the connection created by the connection plugin. Currently what happens in the connection plugin creates a connection and then the napalm-ansible modules ignore this and create their own connection.

Right, so the modules as-is just won't work with a connection plugin, because they're each managing their own connections. The solution, basically, is to remove all of that from each module and replace it with calls to the connection plugin similarly to how the rest of the ansible modules now do it. This should have the side effect of greatly simplifying the modules themselves. For example, the modified napalm_cli I was using while testing the connection is here.

Simpler, that is, as long as you're willing to move to the new connection. If you want to support connection: local and connection: napalm in the same module, this is going to get very complicated very quickly. We managed it in ansible because the connection handling was unified in the action plugin already and only needed to be fixed once per platform, and it still took months of work and is incredibly hacky. Please don't do this.

Also:

2. Modify cli_config and cli_command already in Ansible Core and then no modifications to the NAPALM modules would required

This probably would instead be done through additions to the napalm connection plugin to give it some of the helper methods normally found in a cliconf plugin. Theoretically, this would allow connection: napalm to be used wherever cliconf is expected, in practice I expect it to be a much more complex task.

[ktbyers]

@Qalthos Thanks for the response.

What I meant about Broken handling of username/password is that standard argument passing from the command-line didn't work.

I can probably send over examples, if needed. It has been a while since I tested this, so I don't remember them off the top of the head. I vaguely recollect that something like -u user -k didn't work.

Looking at your code very quickly:

connection = Connection(module._socket_path)

So module._socket_path is just a required reference to the socket_path even though this is entirely unused by napalm? Just making sure I am understanding this correctly.

Thanks for the reference example on how to integrate the connection code.

Simpler, that is, as long as you're willing to move to the new connection. If you want to support connection: local and connection: napalm in the same module, this is going to get very complicated very quickly. 

This is a pretty big issue as it implies everyone using napalm-ansible would have to change (i.e. everyone's current napalm-ansible code would break) and we would have to have an old and new world (i.e. synchronize the napalm-ansible modules that were used with the Ansible version).

We would also probably need to continue maintaining a provider? If that had to go away, that would be a bigger backwards incompatible change.

This is not me saying we should do the it still took months of work and is incredibly hacky just that the other option is to leave things as they currently are (which is probably my preferred solution).

Modify cli_config and cli_command already in Ansible Core and then no modifications to the NAPALM modules would required

Yes, I am pretty certain we don't want to do this (at least I don't).

[Qalthos]

connection = Connection(module._socket_path)

So module._socket_path is just a required reference to the socket_path even though this is entirely unused by napalm? Just making sure I am understanding this correctly.

No. module._socket_path is the path to the socket that Ansible uses to communicate to the persistent connection process... the bit that gets forked so that the connection can outlive the play. The resulting connection object can then act like the real connection, while relaying requests to and returning results from the real connection over that socket. The socket path, meanwhile, is automatically set on the module instance, so you don't need to do anything else to use it.

[ktbyers]

I am going to go ahead and close this.

I am just not seeing enough value in doing this versus the work required to accomplish it (and the ongoing work it would entail).

Kirk