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.

Sign up for GitHub

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

Jump to bottom

Pluribus Networks pn access list ip module #49352

Closed
wants to merge 3 commits into from
Closed

Pluribus Networks pn access list ip module #49352

wants to merge 3 commits into from

Conversation

rajaspachipulusu17
Copy link
Contributor

SUMMARY
ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

pn_access_list_ip module which would add associate ip address to access list.

ADDITIONAL INFORMATION

ansible 2.4.0.0
config file = None
configured module search path = [u'/root/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
ansible python module location = /usr/local/lib/python2.7/dist-packages/ansible
executable location = /usr/local/bin/ansible
python version = 2.7.12 (default, Dec 4 2017, 14:50:18) [GCC 5.4.0 20160609]

@ansibot
Copy link
Contributor

ansibot commented Nov 30, 2018

Hi @rajaspachipulusu17, thank you for submitting this pull-request!

click here for bot help

@ansibot
Copy link
Contributor

ansibot commented Nov 30, 2018

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

lib/ansible/modules/network/netvisor/pn_access_list_ip.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/netvisor/pn_access_list_ip.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/netvisor/pn_access_list_ip.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/netvisor/pn_access_list_ip.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/netvisor/pn_access_list_ip.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/ 
Makefile:93: recipe for target 'modules' failed
--> Standard Error
Traceback (most recent call last):
  File "../bin/plugin_formatter.py", line 720, in <module>
    main()
  File "../bin/plugin_formatter.py", line 678, 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 269, 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 96, 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 382, in parse_block_sequence_entry
    if self.check_token(BlockEntryToken):
  File "/usr/local/lib/python3.6/dist-packages/yaml/scanner.py", line 115, in check_token
    while self.need_more_tokens():
  File "/usr/local/lib/python3.6/dist-packages/yaml/scanner.py", line 149, in need_more_tokens
    self.stale_possible_simple_keys()
  File "/usr/local/lib/python3.6/dist-packages/yaml/scanner.py", line 289, in stale_possible_simple_keys
    "could not find expected ':'", self.get_mark())
yaml.scanner.ScannerError: while scanning a simple key
  in "<unicode string>", line 19, column 7:
          'absent' to remove access-list-ip.
          ^
could not find expected ':'
  in "<unicode string>", line 20, column 5:
        required: True
        ^
make: *** [modules] Error 1

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

lib/ansible/modules/network/netvisor/pn_access_list_ip.py:131:13: E123 closing bracket does not match indentation of opening bracket's line
lib/ansible/modules/network/netvisor/pn_access_list_ip.py:160:22: E126 continuation line over-indented for hanging indent

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

lib/ansible/modules/network/netvisor/pn_access_list_ip.py:0:0: E324 Value for "default" from the argument_spec ('::') for "pn_ip" does not match the documentation (None)
lib/ansible/modules/network/netvisor/pn_access_list_ip.py:0:0: E326 Value for "choices" from the argument_spec (['present', 'absent']) for "state" does not match the documentation ([])
lib/ansible/modules/network/netvisor/pn_access_list_ip.py:33:5: E302 DOCUMENTATION is not valid YAML

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

lib/ansible/modules/network/netvisor/pn_access_list_ip.py:33:5: error DOCUMENTATION: syntax error: could not find expected ':'

click here for bot help

@ansibot
Copy link
Contributor

ansibot commented Nov 30, 2018

cc @Qalthos @amitsi @csharpe-pn @pdam @preetiparasar
click here for bot help

@ansibot
Copy link
Contributor

ansibot commented Nov 30, 2018

@rajaspachipulusu17, 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 ansibot added affects_2.8 This issue/PR affects Ansible v2.8 module This issue/PR relates to a module. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. needs_triage Needs a first human triage before being processed. networking Network category new_module This PR includes a new module. new_plugin This PR includes a new plugin. support:community This issue/PR relates to code supported by the Ansible community. labels Nov 30, 2018
Akasurde
Akasurde requested changes Nov 30, 2018
View reviewed changes
lib/ansible/modules/network/netvisor/pn_access_list_ip.py Outdated
version_added: "2.8"
short_description: CLI command to add/remove access-list-ip
description:
- This modules is used to dd IPs associated with access list
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- This modules is used to dd IPs associated with access list
- This modules is used to add IPs associated with access list

lib/ansible/modules/network/netvisor/pn_access_list_ip.py Outdated
choices: [ "present", "absent"]
pn_ip:
description:
- IP associated with the access list
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- IP associated with the access list
- IP associated with the access list.

lib/ansible/modules/network/netvisor/pn_access_list_ip.py Outdated
default: '::'
pn_name:
description:
- Access List Name
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Access List Name
- Access List Name.

lib/ansible/modules/network/netvisor/pn_access_list_ip.py Outdated
name = module.params['pn_name']
ip = module.params['pn_ip']

show = cli + \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

show = [cli, 'access-list-ip-show', 'name', name, 'format', 'ip', 'no-show-headers'] 
out = module.run_command(show, use_unsafe_shell=True)[1]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok let change and test and send it for review.

Copy link
Contributor Author

@rajaspachipulusu17 rajaspachipulusu17 Nov 30, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Above show which you have suggested wont split my cli parameter.

Hence i have concatenated the string before using it in run_command.

out = module.run_command(cli.split(), use_unsafe_shell=True)[1]

lib/ansible/modules/network/netvisor/pn_access_list_ip.py Outdated
type: bool
"""

import shlex
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

run_command internally uses shlex. You need to pass command as list and use use_unsafe_shell=True. You can find the documentation here -

ansible/lib/ansible/module_utils/basic.py

Line 2702 in 68cafbd

:arg args: is the command to run

@Akasurde
Copy link
Member

@rajaspachipulusu17 Could you please add test case for this change ? Thanks.

@ansibot ansibot removed the needs_triage Needs a first human triage before being processed. label Nov 30, 2018
@ansibot
Copy link
Contributor

ansibot commented Nov 30, 2018

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

lib/ansible/modules/network/netvisor/pn_access_list_ip.py:0:0: E305 DOCUMENTATION.author: Invalid author for dictionary value @ data['author']. Got 'Pluribus Networks (rpachi@pluribusnetworks.com)'

click here for bot help

@ansibot ansibot added the ci_verified Changes made in this PR are causing tests to fail. label Nov 30, 2018
@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Nov 30, 2018
@rajaspachipulusu17
Copy link
Contributor Author

@Akasurde

Unit test cases are ready. Should i create seperate PR or can i send it in same PR?

Thanks,
Rajas

@Akasurde
Copy link
Member

Akasurde commented Dec 6, 2018

@rajaspachipulusu17 if unit tests are related pn_access_list_ip.py then in this same PR. Or else in a separate PR. Thanks.

@ansibot ansibot added the needs_repo This PR no longer has an associated branch as it was removed by the submitter. label Dec 6, 2018
@rajaspachipulusu17
Copy link
Contributor Author

rajaspachipulusu17 commented Dec 6, 2018 •
edited
Loading

closing this PR, as i deleted branch accidentally. new PR is 49604

@ansibot
Copy link
Contributor

ansibot commented Dec 14, 2018

@rajaspachipulusu17 Your branch does not contain a shippable.yml file. Please rebase your branch to trigger running of current tests.

click here for bot help

@ansibot ansibot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_shippable This PR lacks a shippable.yml in its tree. Please rebase your branch to include the latest CI tests. owner_pr This PR is made by the module's maintainer. and removed new_module This PR includes a new module. labels Dec 14, 2018
@ansibot ansibot added stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. and removed needs_shippable This PR lacks a shippable.yml in its tree. Please rebase your branch to include the latest CI tests. labels Dec 14, 2018
@abadger
Copy link
Contributor

abadger commented Dec 14, 2018

Closing in favour of #49604

@abadger abadger closed this Dec 14, 2018
@ansible ansible locked and limited conversation to collaborators Jul 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Akasurde Akasurde Akasurde requested changes

Assignees
No one assigned
Labels
affects_2.8 This issue/PR affects Ansible v2.8 module This issue/PR relates to a module. needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_repo This PR no longer has an associated branch as it was removed by the submitter. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. networking Network category new_plugin This PR includes a new plugin. owner_pr This PR is made by the module's maintainer. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:community This issue/PR relates to code supported by the Ansible community.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rajaspachipulusu17 @ansibot @Akasurde @abadger