PR to start support for Skydive integration with Ansible #50857

justjais · 2019-01-14T10:06:12Z

SUMMARY

Skydive is a network analyzer tool, which helps in analyzing SDNs and network and with this PR Ansible API and lookup modules are added to query Skydive objects using skydive python client. Opened new PR closing the earlier skydive PR #50136

ISSUE TYPE

New Module Pull Request

COMPONENT NAME

skydive

ADDITIONAL INFORMATION

ansibot · 2019-01-14T10:07:09Z

@justjais this PR contains more than one new module.

Please submit only one new module per pull request. For a detailed explanation, please read the grouped modules documentation

click here for bot help

ansibot · 2019-01-14T10:07:09Z

@justjais, 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

rcarrillocruz

The skydive module name seems odd to me.
You name it 'add_node', yet you provide a state present/absent.

Why not calling it 'skydive_node', which then manages the lifecycle of the skydive 'object' ?

ansibot · 2019-01-14T10:31:30Z

The test ansible-test sanity --test ansible-doc --python 2.6 [explain] failed with 1 error:

lib/ansible/modules/network/skydive/skydive_capture.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 2.7 [explain] failed with 1 error:

lib/ansible/modules/network/skydive/skydive_capture.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 3.5 [explain] failed with 1 error:

lib/ansible/modules/network/skydive/skydive_capture.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 3.6 [explain] failed with 1 error:

lib/ansible/modules/network/skydive/skydive_capture.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 3.7 [explain] failed with 1 error:

lib/ansible/modules/network/skydive/skydive_capture.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test docs-build [explain] failed with the error:

Command "/usr/bin/python test/sanity/code-smell/docs-build.py" returned exit status 1.
>>> Standard Error
Command 'make singlehtmldocs' failed with status code: 2
--> Standard Output
cat _themes/srtd/static/css/theme.css | sed -e 's/^[ 	]*//g; s/[ 	]*$//g; s/\([:{;,]\) /\1/g; s/ {/{/g; s/\/\*.*\*\///g; /^$/d' | sed -e :a -e '$!N; s/\n\(.\)/\1/; ta' > _themes/srtd/static/css/theme.min.css
PYTHONPATH=../../lib ../bin/dump_config.py --template-file=../templates/config.rst.j2 --output-dir=rst/reference_appendices/ -d ../../lib/ansible/config/base.yml
mkdir -p rst/cli
PYTHONPATH=../../lib ../bin/generate_man.py --template-file=../templates/cli_rst.j2 --output-dir=rst/cli/ --output-format rst ../../lib/ansible/cli/*.py
PYTHONPATH=../../lib ../bin/dump_keywords.py --template-dir=../templates --output-dir=rst/reference_appendices/ -d ./keyword_desc.yml
PYTHONPATH=../../lib ../bin/plugin_formatter.py -t rst --template-dir=../templates --module-dir=../../lib/ansible/modules -o rst/modules/ 
Evaluating module files...
Makefile:93: recipe for target 'modules' failed
--> Standard Error
Traceback (most recent call last):
  File "../bin/plugin_formatter.py", line 754, in <module>
    main()
  File "../bin/plugin_formatter.py", line 709, in main
    plugin_info, categories = get_plugin_info(options.module_dir, limit_to=options.limit_to, verbose=(options.verbosity > 0))
  File "../bin/plugin_formatter.py", line 294, in get_plugin_info
    doc, examples, returndocs, metadata = plugin_docs.get_docstring(module_path, fragment_loader, verbose=verbose)
  File "/root/ansible/lib/ansible/utils/plugin_docs.py", line 103, in get_docstring
    data = read_docstring(filename, verbose=verbose, ignore_errors=ignore_errors)
  File "/root/ansible/lib/ansible/parsing/plugin_docs.py", line 59, in read_docstring
    data[varkey] = AnsibleLoader(child.value.s, file_name=filename).get_single_data()
  File "/usr/local/lib/python3.6/dist-packages/yaml/constructor.py", line 35, in get_single_data
    node = self.get_single_node()
  File "/usr/local/lib/python3.6/dist-packages/yaml/composer.py", line 36, in get_single_node
    document = self.compose_document()
  File "/usr/local/lib/python3.6/dist-packages/yaml/composer.py", line 55, in compose_document
    node = self.compose_node(None, None)
  File "/usr/local/lib/python3.6/dist-packages/yaml/composer.py", line 84, in compose_node
    node = self.compose_mapping_node(anchor)
  File "/usr/local/lib/python3.6/dist-packages/yaml/composer.py", line 133, in compose_mapping_node
    item_value = self.compose_node(node, item_key)
  File "/usr/local/lib/python3.6/dist-packages/yaml/composer.py", line 84, in compose_node
    node = self.compose_mapping_node(anchor)
  File "/usr/local/lib/python3.6/dist-packages/yaml/composer.py", line 133, in compose_mapping_node
    item_value = self.compose_node(node, item_key)
  File "/usr/local/lib/python3.6/dist-packages/yaml/composer.py", line 84, in compose_node
    node = self.compose_mapping_node(anchor)
  File "/usr/local/lib/python3.6/dist-packages/yaml/composer.py", line 133, in compose_mapping_node
    item_value = self.compose_node(node, item_key)
  File "/usr/local/lib/python3.6/dist-packages/yaml/composer.py", line 82, in compose_node
    node = self.compose_sequence_node(anchor)
  File "/usr/local/lib/python3.6/dist-packages/yaml/composer.py", line 110, in compose_sequence_node
    while not self.check_event(SequenceEndEvent):
  File "/usr/local/lib/python3.6/dist-packages/yaml/parser.py", line 98, in check_event
    self.current_event = self.state()
  File "/usr/local/lib/python3.6/dist-packages/yaml/parser.py", line 393, in parse_block_sequence_entry
    "expected <block end>, but found %r" % token.id, token.start_mark)
yaml.parser.ParserError: while parsing a block collection
  in "<unicode string>", line 27, column 7:
          - Configures a text string to be ... 
          ^
expected <block end>, but found '?'
  in "<unicode string>", line 29, column 7:
          default: ''
          ^
make: *** [modules] Error 1

The test ansible-test sanity --test validate-modules [explain] failed with 7 errors:

lib/ansible/modules/network/skydive/skydive_add_node.py:0:0: E322 "provider" is listed in the argument_spec, but not documented in the module
lib/ansible/modules/network/skydive/skydive_capture.py:0:0: E324 Value for "default" from the argument_spec ('False') for "extra_tcp_metric" does not match the documentation (None)
lib/ansible/modules/network/skydive/skydive_capture.py:0:0: E324 Value for "default" from the argument_spec ('False') for "ip_defrag" does not match the documentation (None)
lib/ansible/modules/network/skydive/skydive_capture.py:0:0: E324 Value for "default" from the argument_spec ('False') for "reassemble_tcp" does not match the documentation (None)
lib/ansible/modules/network/skydive/skydive_capture.py:0:0: E324 Value for "default" from the argument_spec ('present') for "state" does not match the documentation (None)
lib/ansible/modules/network/skydive/skydive_capture.py:0:0: E326 Value for "choices" from the argument_spec (['present', 'absent']) for "state" does not match the documentation ([])
lib/ansible/modules/network/skydive/skydive_capture.py:43:7: E302 DOCUMENTATION is not valid YAML

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

lib/ansible/modules/network/skydive/skydive_capture.py:43:7: error DOCUMENTATION: syntax error: expected <block end>, but found '?'

click here for bot help

ansibot · 2019-01-17T08:57:52Z

@justjais this PR contains the following merge commits:

9384c64

Please rebase your branch to remove these commits.

click here for bot help

ansibot · 2019-01-17T10:04:04Z

The test ansible-test sanity --test ansible-doc --python 2.6 [explain] failed with 1 error:

lib/ansible/plugins/lookup/skydive_lookup.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 2.7 [explain] failed with 1 error:

lib/ansible/plugins/lookup/skydive_lookup.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 3.5 [explain] failed with 1 error:

lib/ansible/plugins/lookup/skydive_lookup.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 3.7 [explain] failed with 1 error:

lib/ansible/plugins/lookup/skydive_lookup.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 3.6 [explain] failed with 1 error:

lib/ansible/plugins/lookup/skydive_lookup.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test docs-build [explain] failed with 10 errors:

docs/docsite/rst/plugins/connection.rst:39:0: undefined-label: undefined label: inventory_hostnames_lookup (if the link has no caption the label must precede a section header)
docs/docsite/rst/plugins/lookup.rst:49:0: undefined-label: undefined label: items_lookup (if the link has no caption the label must precede a section header)
docs/docsite/rst/plugins/lookup.rst:144:0: toc-tree-glob-pattern-no-match: toctree glob pattern 'lookup/*' didn't match any documents
docs/docsite/rst/reference_appendices/faq.rst:606:0: undefined-label: undefined label: vars_lookup (if the link has no caption the label must precede a section header)
docs/docsite/rst/scenario_guides/guide_infoblox.rst:46:0: undefined-label: undefined label: nios_lookup (if the link has no caption the label must precede a section header)
docs/docsite/rst/scenario_guides/guide_infoblox.rst:47:0: undefined-label: undefined label: nios_next_ip_lookup (if the link has no caption the label must precede a section header)
docs/docsite/rst/scenario_guides/guide_infoblox.rst:48:0: undefined-label: undefined label: nios_next_network_lookup (if the link has no caption the label must precede a section header)
docs/docsite/rst/scenario_guides/guide_infoblox.rst:56:0: undefined-label: undefined label: nios_lookup (if the link has no caption the label must precede a section header)
docs/docsite/rst/scenario_guides/guide_infoblox.rst:156:0: undefined-label: undefined label: nios_lookup (if the link has no caption the label must precede a section header)
docs/docsite/rst/scenario_guides/guide_infoblox.rst:208:0: undefined-label: undefined label: nios_next_ip_lookup (if the link has no caption the label must precede a section header)

The test ansible-test sanity --test pep8 [explain] failed with 2 errors:

lib/ansible/plugins/lookup/skydive_lookup.py:42:56: W291 trailing whitespace
lib/ansible/plugins/lookup/skydive_lookup.py:47:12: W291 trailing whitespace

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

lib/ansible/plugins/lookup/skydive_lookup.py:38:5: error DOCUMENTATION: syntax error: could not find expected ':'

click here for bot help

ansibot · 2019-01-18T10:29:07Z

The test ansible-test sanity --test pep8 [explain] failed with 2 errors:

lib/ansible/module_utils/network/skydive/skydive.py:54:1: E302 expected 2 blank lines, found 1
lib/ansible/plugins/lookup/skydive.py:46:98: W291 trailing whitespace

click here for bot help

justjais · 2019-01-25T13:06:50Z

@safchain @pierrecregut please review the respective PR.

safchain · 2019-01-25T13:24:32Z

Adding @lebauce who is core dev of Skydive

lib/ansible/module_utils/network/skydive/skydive.py

pierrecregut · 2019-01-28T10:32:00Z

lib/ansible/module_utils/network/skydive/skydive.py

+        super(skydive_lookup, self).__init__(host, username, password)
+        self.query_str = ""
+
+    def lookup_query(self, filter_data):


Limited to nodes and limited to a single Has filter. I would not constrain the gremlin query that can be sent. If you want to
provide a simpler lookup for people who do not want to understand gremlin, provide it as an additional query. Example of things that are useful in practice : Find the interface node (type "interface") named "eth0' that is a child of a host (type "host") that is named "my_machine". When you glue nodes created by ansible and nodes found by skydive, you need at least this expressive power in your lookup queries.

cool, got the use case and shall raise a new PR supporting the chained queries.

I think that the filtering mechanism is a bit limited, maybe adding a chained queries will help but as Pierre suggested I would prefer to let the user uses a raw gremlin expression as they can be complex returning either nodes or edges, few examples :

G.V().Has('Type', 'veth', 'Name', 'eth0').In().Has('Name', 'vm1') G.V().HasEither('Type', 'ovsbridge', 'Name', 'br-ex').Out('State', NE('UP')) G.V().HasKey('MTU') G.E().Has('RelationType', 'layer2').InV().Has('Name', 'en0')

A more complete Skydive gremlin documentation here :
http://skydive.network/documentation/api-gremlin

Do you have an example of how such request will be handled with the chained queries ?

lib/ansible/module_utils/network/skydive/skydive.py

pierrecregut · 2019-01-28T10:32:28Z

lib/ansible/modules/network/skydive/skydive_node.py

+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+from __future__ import absolute_import, division, print_function
+__metaclass__ = type


use six.add_metaclass for python3 compability tells flake8.

lib/ansible/modules/network/skydive/skydive_node.py

pierrecregut · 2019-01-28T10:33:13Z

lib/ansible/utils/module_docs_fragments/skydive.py

+    suboptions:
+      host:
+        description:
+          - Specifies the host name or address for connecting to the remote


Name is not clear and description does not really help to understand the format hostname:port

updated the description to better give the idea of hostname:port

pierrecregut · 2019-01-28T10:52:02Z

I will add some more generic feedback on how to add nodes and edges to skydive and why some items are necessary.

complex node lookup (I already explained why).
WSClient host_id : there can only be one connection with a given host_id at a point in time. As by default ansible parallelizes tasks, you need to provide different host_ids.
We need to give some control on the uuid in use. When you have a hierarchy of nodes with different levels, you often have items at the low level with the same name (eg: two interfaces "eth0" but on different hosts, or even on different cards of the same host, etc), if you use only your hostname (something like 'ansible') and the element name, you will have uuid collisions.
Creating a new websocket for each new node can be very slow if you have a lot of nodes. If you have an inventory manager, you probably have a way to get your complete switch topology with only one request, then instantiating it in Skydive may take half an hour on a moderate deployment. Using batch specification where you reuse the websocket can speed up node creation (in my case the full script had a 25x speed improvement)
Having a lookup could replace the option in current skydive ansible module where you give gremlin queries as parameters to skydive_edge. But in this case, we probably need shortcuts to get directly the uuid and get one and only one uuid from a query without dozen lines of map and other weird filters.

ansibot · 2019-02-13T04:53:11Z

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

lib/ansible/module_utils/network/skydive/api.py:63:12: unreachable Unreachable code

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

lib/ansible/plugins/lookup/skydive.py:42:66: error EXAMPLES: syntax error: expected <block end>, but found '<scalar>'

click here for bot help

Signed-off-by: Sumit Jaiswal <sjaiswal@redhat.com>

ansibot · 2019-02-13T06:07:31Z

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

lib/ansible/plugins/lookup/skydive.py:42:66: error EXAMPLES: syntax error: expected <block end>, but found '<scalar>'

click here for bot help

justjais · 2019-02-13T07:21:56Z

@safchain @pierrecregut Apologies for the delay in updating the code with the review comments as I was on PTO, I have implemented the suggested review comments. Kindly, review and let me know if it's ok to merge.

Also, as discussed earlier I shall raise different PR for supporting skydive edge and node modules.

lib/ansible/module_utils/network/skydive/api.py

Signed-off-by: Sumit Jaiswal <sjaiswal@redhat.com>

ansibot · 2019-02-14T14:04:59Z

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

lib/ansible/module_utils/network/skydive/api.py:129:81: E126 continuation line over-indented for hanging indent

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

lib/ansible/plugins/lookup/skydive.py:42:66: error EXAMPLES: syntax error: expected <block end>, but found '<scalar>'

click here for bot help

Signed-off-by: Sumit Jaiswal <sjaiswal@redhat.com>

lib/ansible/plugins/lookup/skydive.py

Signed-off-by: Sumit Jaiswal <sjaiswal@redhat.com>

justjais requested a review from privateip January 14, 2019 10:06

rcarrillocruz reviewed Jan 14, 2019

View reviewed changes

justjais changed the title ~~Skydive mod~~ PR to start support for Skydive integration with Ansible Jan 14, 2019

rcarrillocruz approved these changes Jan 17, 2019

View reviewed changes

ansibot added merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html labels Jan 17, 2019

justjais force-pushed the skydive_mod branch from 829de7e to 819a2e5 Compare January 17, 2019 09:16

ansibot removed merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html labels Jan 17, 2019

ansibot added the support:core This issue/PR relates to code supported by the Ansible Engineering Team. label Jan 25, 2019

justjais mentioned this pull request Jan 28, 2019

Skydive network analyser integration with Ansible via modules #50091

Closed

ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jan 28, 2019

pierrecregut suggested changes Jan 28, 2019

View reviewed changes

ansibot removed merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html labels Feb 13, 2019

ansibot added the ci_verified Changes made in this PR are causing tests to fail. label Feb 13, 2019

justjais added 2 commits February 13, 2019 10:56

fix shippable errors

8155726

Signed-off-by: Sumit Jaiswal <sjaiswal@redhat.com>

fix shippable error

bfc7fd2

Signed-off-by: Sumit Jaiswal <sjaiswal@redhat.com>

ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Feb 13, 2019

pierrecregut suggested changes Feb 13, 2019

View reviewed changes

lib/ansible/module_utils/network/skydive/api.py Outdated Show resolved Hide resolved

lib/ansible/module_utils/network/skydive/api.py Outdated Show resolved Hide resolved

lib/ansible/module_utils/network/skydive/api.py Outdated Show resolved Hide resolved

mattclay added the ci_verified Changes made in this PR are causing tests to fail. label Feb 14, 2019

justjais added 3 commits February 14, 2019 18:44

fix review comments

38f676d

Signed-off-by: Sumit Jaiswal <sjaiswal@redhat.com>

fixing shippable errors

34bcab1

Signed-off-by: Sumit Jaiswal <sjaiswal@redhat.com>

fix review comments

7d9ec75

Signed-off-by: Sumit Jaiswal <sjaiswal@redhat.com>

ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Feb 14, 2019

mattclay added the ci_verified Changes made in this PR are causing tests to fail. label Feb 14, 2019

justjais added 2 commits February 15, 2019 13:19

review comments

9c5a0bb

Signed-off-by: Sumit Jaiswal <sjaiswal@redhat.com>

shippable fix

38685e2

Signed-off-by: Sumit Jaiswal <sjaiswal@redhat.com>

ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Feb 15, 2019

safchain reviewed Feb 19, 2019

View reviewed changes

lib/ansible/plugins/lookup/skydive.py Show resolved Hide resolved

review comment

eaad57f

Signed-off-by: Sumit Jaiswal <sjaiswal@redhat.com>

safchain approved these changes Feb 19, 2019

View reviewed changes

justjais merged commit 8a2ff1c into ansible:devel Feb 19, 2019

sivel removed the needs_triage Needs a first human triage before being processed. label Feb 19, 2019

justjais deleted the skydive_mod branch February 20, 2019 04:16

ansible locked and limited conversation to collaborators Jul 25, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PR to start support for Skydive integration with Ansible #50857

PR to start support for Skydive integration with Ansible #50857

justjais commented Jan 14, 2019

ansibot commented Jan 14, 2019

ansibot commented Jan 14, 2019

rcarrillocruz left a comment

ansibot commented Jan 14, 2019

ansibot commented Jan 17, 2019

ansibot commented Jan 17, 2019

ansibot commented Jan 18, 2019

justjais commented Jan 25, 2019

safchain commented Jan 25, 2019

pierrecregut Jan 28, 2019

justjais Jan 28, 2019

safchain Jan 28, 2019

pierrecregut Jan 28, 2019

pierrecregut Jan 28, 2019

justjais Jan 28, 2019

pierrecregut commented Jan 28, 2019

ansibot commented Feb 13, 2019

ansibot commented Feb 13, 2019

justjais commented Feb 13, 2019 •

edited

ansibot commented Feb 14, 2019

PR to start support for Skydive integration with Ansible #50857

PR to start support for Skydive integration with Ansible #50857

Conversation

justjais commented Jan 14, 2019

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

ansibot commented Jan 14, 2019

ansibot commented Jan 14, 2019

rcarrillocruz left a comment

Choose a reason for hiding this comment

ansibot commented Jan 14, 2019

ansibot commented Jan 17, 2019

ansibot commented Jan 17, 2019

ansibot commented Jan 18, 2019

justjais commented Jan 25, 2019

safchain commented Jan 25, 2019

pierrecregut Jan 28, 2019

Choose a reason for hiding this comment

justjais Jan 28, 2019

Choose a reason for hiding this comment

safchain Jan 28, 2019

Choose a reason for hiding this comment

pierrecregut Jan 28, 2019

Choose a reason for hiding this comment

pierrecregut Jan 28, 2019

Choose a reason for hiding this comment

justjais Jan 28, 2019

Choose a reason for hiding this comment

pierrecregut commented Jan 28, 2019

ansibot commented Feb 13, 2019

ansibot commented Feb 13, 2019

justjais commented Feb 13, 2019 • edited

ansibot commented Feb 14, 2019

justjais commented Feb 13, 2019 •

edited