Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GRPC_PLUG] Add gRPC connection plugin support to work with network os supporting gRPC #52823

Closed
wants to merge 13 commits into from

Conversation

@ganeshrn
Copy link
Member

@ganeshrn ganeshrn commented Feb 22, 2019

Fixes #38820

SUMMARY
  • This PR adds gRPC connection plugin.
  • Currently it uses ansible_network_os to load the correct
    gRPC implementation plugins (will be added in subsequent PR's).
    TODO: In future need to add support to work with non-network use case
ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

lib/ansible/plugins/connection/grpc.py

ADDITIONAL INFORMATION

@ganeshrn ganeshrn force-pushed the ganeshrn:grpc_connection_plugin branch 2 times, most recently Feb 23, 2019
@ganeshrn ganeshrn requested a review from samccann Feb 23, 2019
@ganeshrn ganeshrn force-pushed the ganeshrn:grpc_connection_plugin branch Feb 26, 2019
@ansibot
Copy link
Contributor

@ansibot ansibot commented Feb 26, 2019

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

lib/ansible/modules/cloud/kubevirt/kubevirt_template.py:0:0: E325 Argument 'resource_definition' in argument_spec defines type as <function list_dict_str at 0x7f292cd2bc80> but documentation defines type as 'str'

click here for bot help

@Qalthos
Qalthos approved these changes Mar 4, 2019
@ansibot
Copy link
Contributor

@ansibot ansibot commented Mar 4, 2019

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

lib/ansible/plugins/connection/grpc.py:261:34: E127 continuation line over-indented for visual indent

click here for bot help

@ganeshrn ganeshrn force-pushed the ganeshrn:grpc_connection_plugin branch Mar 5, 2019
@ansibot
Copy link
Contributor

@ansibot ansibot commented Mar 5, 2019

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

lib/ansible/plugins/connection/grpc.py:262:34: E127 continuation line over-indented for visual indent

click here for bot help

Fixes #38820

* This PR adds gRPC connection plugin.
* Currently it uses `ansible_network_os` to load the correct
  gRPC implementation plugins (will be added in subsequent PR's).
  TODO: In future need to expand it to work with non-network use case
@ganeshrn ganeshrn force-pushed the ganeshrn:grpc_connection_plugin branch to 7818a6b Mar 5, 2019
@ikhan2010 ikhan2010 added this to Needs Triage in Networking via automation Apr 1, 2019
@ikhan2010 ikhan2010 moved this from Needs Triage to In Progress in Networking Apr 1, 2019
@ikhan2010 ikhan2010 changed the title Add gRPC connection plugin support to work with network os supporting gRPC [GRPC_PLUG] Add gRPC connection plugin support to work with network os supporting gRPC Apr 1, 2019
@nitzmahone
Copy link
Member

@nitzmahone nitzmahone commented Jul 15, 2019

@ganeshrn Jimi and I were also back-channeling on what it would take to build a "swap-a-dee-doo stub connection plugin" (patent pending ;) ) that would delegate to another connection at construction or later. That way, all grpc connections would be fully-fledged connection plugins that could be shipped, documented, and referenced as such, but you could have the stub actually substitute (ideally) or proxy (less ideally) the right one based on ansible_network_os without needing to create a new plugin type at all. Doing that all inside the controller would be relatively simple, but as with several other things, the introduction of ansible-connection significantly complicates it...

@ganeshrn
Copy link
Member Author

@ganeshrn ganeshrn commented Jul 16, 2019

@nitzmahone That certainly sounds like a patentable idea :). Would like to know more details on this approach.

stub actually substitute (ideally) or proxy (less ideally) the right one based on ansible_network_os without needing to create a new plugin type at all.

How the stub would work to invoke the right grpc connection (sub-plugin in current form). Also with this approach can it support the configurable grpc plugin path option functionality?

In case of the private plugin loader per connection approach that we discussed internally how the directory structure would look like?

In case of ansible/ansible

$tree lib/ansible/plugins/connection
.
├── __init__.py
├── grpc
 │   ├── __init__.py
 │   └── xos.py
├── grpc.py

For collections:

$tree network
.
├── collections
│   └── ansible_collections
│       └── mycoll
│           └── coll
│               └── plugins
│                   ├── connection
│                   │   ├── grpc
│                   │   │   └── xos.py
│                   │   └── grpc.py
@abenokraitis
Copy link
Contributor

@abenokraitis abenokraitis commented Feb 3, 2020

Might be best to put this into its own collection going forward.

@wisotzky
Copy link
Contributor

@wisotzky wisotzky commented Feb 17, 2020

@abenokraitis, while it should be possible to move this code into a collection, at least in Ansible 2.9 this is not yet possible.
Following dependencies do exist, based on the contribution from @ganeshrn:

  • constants.py (part of Ansible Base) lists the CONFIGURABLE_PLUGINS
  • config/base.yml' (part of Ansible Base) defines the DEFAULT_xxx_PLUGIN_PATH`
  • plugins/loader.py' (part of Ansible Base) instantiates the grpc_loadderof classPluginLoader`

To allow adding new plugin-types using collections, Ansible Base would need to updated to have a mechanism to avoid hardcoding of plugin artefacts but to make it loadable at runtime.

@wisotzky
Copy link
Contributor

@wisotzky wisotzky commented Feb 17, 2020

@ganeshrn, it is worth to revisit the mechanics that have been implemented in this PR.

BACKGROUND
Devices that support gRPC APIs, might support one or more gRPC services. Beside of vendor-specific gRPC services, there are common services like OpenConfig gNMI and gNOI that are commonly supported by a growing number of vendors.

REQUIREMENTS
As a conclusion, I would see the following requirements for the gRPC plugin framework:

  1. There should be just one sub-plugin for gRPC services like "OpenConfig gNMI" while it must be avoided to define the gNMI sub-plugin over and over again per network_os.
  2. Exposing common methods in class GrpcBase like Set or Get does not make much sense. For example gNOI is all about device operations but not configuration. If there would be a common, abstracted method for gRPC it would be execute_rpc() that consumes the name of the RPC and generic payload.
  3. If a device supports multiple gRPC services like gNMI and gNOI only one single underlying persistent gRPC/TLS connection shall be used and shared for both services.
  4. To load gRPC sub-plugins shall be rather independent of the network_os. It is suggested, that Ansible modules trigger to load sub plugins as needed. For example, if the playbook calls the module gnmi_get Ansible should load the gnmi sub-plugin automatically.
  5. As a matter of consequence, there would be no common gRPC-related Ansible modules like grpc_config (similar to cli_config or netconf_config). Instead the, Ansible modules would always be specific to the underlying gRPC service resulting in modules like gnmi_config.

NEXT STEPS
I've almost finished the implementation of the bespoken principles/requirements for gRPC. I would create a new PR for this. Still work-in-progress to build the corresponding collections for gNMI and gNOI.

@ganeshrn
Copy link
Member Author

@ganeshrn ganeshrn commented Jun 28, 2021

@ganeshrn ganeshrn closed this Jun 28, 2021
@ansible ansible locked and limited conversation to collaborators Jul 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.