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

Added Exos vlan resource module #61865

Open
wants to merge 12 commits into
base: devel
from

Conversation

@JayalakshmiV
Copy link

commented Sep 5, 2019

SUMMARY

Resource module for exos_vlan, with supported operations and integration tests.

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

exos_vlan

ADDITIONAL INFORMATION

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Sep 5, 2019

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

@JayalakshmiV JayalakshmiV force-pushed the JayalakshmiV:exos_vlan branch from 50467d8 to 7c80ef0 Sep 5, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Sep 5, 2019

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

lib/ansible/modules/network/exos/exos_vlans.py:0:0: module-incorrect-version-added: version_added should be '2.10'. Currently 2.9
lib/ansible/modules/network/exos/exos_vlans.py:0:0: return-syntax-error: RETURN.after.type: required key not provided @ data['after']['type']. Got None
lib/ansible/modules/network/exos/exos_vlans.py:0:0: return-syntax-error: RETURN.before.type: required key not provided @ data['before']['type']. Got None
lib/ansible/modules/network/exos/exos_vlans.py:756:0: missing-if-name-main: Next to last line should be: if __name__ == "__main__":

The test ansible-test sanity --test pylint [explain] failed with 16 errors:

lib/ansible/module_utils/network/exos/config/vlans/vlans.py:45:60: trailing-whitespace: Trailing whitespace
lib/ansible/module_utils/network/exos/config/vlans/vlans.py:46:5: trailing-whitespace: Trailing whitespace
lib/ansible/module_utils/network/exos/config/vlans/vlans.py:56:32: bad-whitespace: No space allowed before comma         "config": {"name": None , "status": "ACTIVE" , "tpid": "oc-vlan-types:TPID_0x8100", "vlan-id": None }                                 ^
lib/ansible/module_utils/network/exos/config/vlans/vlans.py:56:53: bad-whitespace: No space allowed before comma         "config": {"name": None , "status": "ACTIVE" , "tpid": "oc-vlan-types:TPID_0x8100", "vlan-id": None }                                                      ^
lib/ansible/module_utils/network/exos/config/vlans/vlans.py:56:108: bad-whitespace: No space allowed before bracket         "config": {"name": None , "status": "ACTIVE" , "tpid": "oc-vlan-types:TPID_0x8100", "vlan-id": None }                                                                                                             ^
lib/ansible/module_utils/network/exos/config/vlans/vlans.py:92:0: trailing-whitespace: Trailing whitespace
lib/ansible/module_utils/network/exos/config/vlans/vlans.py:134:0: trailing-whitespace: Trailing whitespace
lib/ansible/module_utils/network/exos/config/vlans/vlans.py:158:86: trailing-whitespace: Trailing whitespace
lib/ansible/module_utils/network/exos/config/vlans/vlans.py:165:0: trailing-whitespace: Trailing whitespace
lib/ansible/module_utils/network/exos/config/vlans/vlans.py:192:86: trailing-whitespace: Trailing whitespace
lib/ansible/module_utils/network/exos/config/vlans/vlans.py:195:0: trailing-whitespace: Trailing whitespace
lib/ansible/module_utils/network/exos/config/vlans/vlans.py:205:0: trailing-whitespace: Trailing whitespace
lib/ansible/module_utils/network/exos/config/vlans/vlans.py:229:49: trailing-whitespace: Trailing whitespace
lib/ansible/module_utils/network/exos/config/vlans/vlans.py:269:0: trailing-newlines: Trailing newlines
lib/ansible/module_utils/network/exos/facts/vlans/vlans.py:87:0: trailing-newlines: Trailing newlines
lib/ansible/modules/network/exos/exos_vlans.py:757:0: trailing-newlines: Trailing newlines

The test ansible-test sanity --test future-import-boilerplate [explain] failed with 4 errors:

lib/ansible/module_utils/network/exos/argspec/vlans/vlans.py:0:0: missing: from __future__ import (absolute_import, division, print_function)
lib/ansible/module_utils/network/exos/config/vlans/vlans.py:0:0: missing: from __future__ import (absolute_import, division, print_function)
lib/ansible/module_utils/network/exos/facts/vlans/vlans.py:0:0: missing: from __future__ import (absolute_import, division, print_function)
lib/ansible/module_utils/network/exos/utils/utils.py:0:0: missing: from __future__ import (absolute_import, division, print_function)

The test ansible-test sanity --test metaclass-boilerplate [explain] failed with 4 errors:

lib/ansible/module_utils/network/exos/argspec/vlans/vlans.py:0:0: missing: __metaclass__ = type
lib/ansible/module_utils/network/exos/config/vlans/vlans.py:0:0: missing: __metaclass__ = type
lib/ansible/module_utils/network/exos/facts/vlans/vlans.py:0:0: missing: __metaclass__ = type
lib/ansible/module_utils/network/exos/utils/utils.py:0:0: missing: __metaclass__ = type

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

lib/ansible/module_utils/network/exos/argspec/vlans/vlans.py:38:13: E126: continuation line over-indented for hanging indent
lib/ansible/module_utils/network/exos/config/vlans/vlans.py:22:1: E302: expected 2 blank lines, found 1
lib/ansible/module_utils/network/exos/config/vlans/vlans.py:43:41: E231: missing whitespace after ':'
lib/ansible/module_utils/network/exos/config/vlans/vlans.py:45:61: W291: trailing whitespace
lib/ansible/module_utils/network/exos/config/vlans/vlans.py:46:6: W291: trailing whitespace
lib/ansible/module_utils/network/exos/config/vlans/vlans.py:56:32: E203: whitespace before ','
lib/ansible/module_utils/network/exos/config/vlans/vlans.py:56:53: E203: whitespace before ','
lib/ansible/module_utils/network/exos/config/vlans/vlans.py:56:108: E202: whitespace before '}'
lib/ansible/module_utils/network/exos/config/vlans/vlans.py:57:9: E123: closing bracket does not match indentation of opening bracket's line
lib/ansible/module_utils/network/exos/config/vlans/vlans.py:92:1: W293: blank line contains whitespace
lib/ansible/module_utils/network/exos/config/vlans/vlans.py:134:1: W293: blank line contains whitespace
lib/ansible/module_utils/network/exos/config/vlans/vlans.py:158:87: W291: trailing whitespace
lib/ansible/module_utils/network/exos/config/vlans/vlans.py:165:1: W293: blank line contains whitespace
lib/ansible/module_utils/network/exos/config/vlans/vlans.py:192:87: W291: trailing whitespace
lib/ansible/module_utils/network/exos/config/vlans/vlans.py:195:1: W293: blank line contains whitespace
lib/ansible/module_utils/network/exos/config/vlans/vlans.py:205:1: W293: blank line contains whitespace
lib/ansible/module_utils/network/exos/config/vlans/vlans.py:229:50: W291: trailing whitespace
lib/ansible/module_utils/network/exos/config/vlans/vlans.py:269:1: W391: blank line at end of file
lib/ansible/module_utils/network/exos/facts/vlans/vlans.py:51:17: E123: closing bracket does not match indentation of opening bracket's line
lib/ansible/module_utils/network/exos/facts/vlans/vlans.py:87:1: W391: blank line at end of file
lib/ansible/module_utils/network/exos/utils/utils.py:9:1: E302: expected 2 blank lines, found 1
lib/ansible/modules/network/exos/exos_vlans.py:184:8: W291: trailing whitespace
lib/ansible/modules/network/exos/exos_vlans.py:334:2: W291: trailing whitespace
lib/ansible/modules/network/exos/exos_vlans.py:740:1: E302: expected 2 blank lines, found 1
lib/ansible/modules/network/exos/exos_vlans.py:757:1: W391: blank line at end of file

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

lib/ansible/module_utils/network/exos/utils/utils.py:0:0: should not have a shebang

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

test/integration/targets/exos_vlan/vars/main.yaml:113:1: empty-lines: too many blank lines (1 > 0)

click here for bot help

@ansibot ansibot added the ci_verified label Sep 5, 2019

DOCUMENTATION = """
---
module: exos_vlans
version_added: 2.9

This comment has been minimized.

Copy link
@ujwalkomarla

ujwalkomarla Sep 5, 2019

Contributor

version_added: 2.10

sample: >
The configuration returned will always be in the same format
of the parameters above.
commands:

This comment has been minimized.

Copy link
@ujwalkomarla

ujwalkomarla Sep 5, 2019

Contributor

Please replace commands section with this:

requests:
  description: The set of requests pushed to the remote device.
  returned: always
  type: list
  sample: [{"data": "...", "method": "...", "path": "..."}, {"data": "...", "method": "...", "path": "..."}, {"data": "...", "method": "...", "path": "..."}]
@@ -0,0 +1,3 @@
---

This comment has been minimized.

Copy link
@ujwalkomarla

ujwalkomarla Sep 5, 2019

Contributor

Please move the 'exos_vlan' target folder to 'exos_vlans' folder.

@ansibot ansibot removed the needs_triage label Sep 5, 2019

@ansibot ansibot removed the ci_verified label Sep 5, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Sep 5, 2019

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

lib/ansible/modules/network/exos/exos_vlans.py:0:0: module-incorrect-version-added: version_added should be '2.10'. Currently 2.1

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

lib/ansible/module_utils/network/exos/config/vlans/vlans.py:95:0: trailing-whitespace: Trailing whitespace

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

lib/ansible/module_utils/network/exos/config/vlans/vlans.py:95:1: W293: blank line contains whitespace

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

lib/ansible/module_utils/network/exos/utils/utils.py:0:0: should not have a shebang

click here for bot help

@ansibot ansibot added the ci_verified label Sep 5, 2019

@ansibot ansibot removed the ci_verified label Sep 5, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Sep 5, 2019

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

lib/ansible/modules/network/exos/exos_vlans.py:0:0: module-incorrect-version-added: version_added should be '2.10'. Currently 2.1

click here for bot help

@ansibot ansibot added the ci_verified label Sep 5, 2019

@ansibot ansibot removed the ci_verified label Sep 5, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Sep 5, 2019

@LindsayHill @bigmstone @hlrichardson @rdvencioneck @ujwalkomarla

As a maintainer of a module in the same namespace this new module has been submitted to, your vote counts for shipits. Please review this module and add shipit if you would like to see it merged.

click here for bot help

- name: Merge the provided configuration with the existing running configuration
exos_vlans: &merged
config:
- name: vlan_30

This comment has been minimized.

Copy link
@ujwalkomarla

ujwalkomarla Sep 6, 2019

Contributor

Add some vlans whose state will be suspended in the merged, overridden, replaced tests.


if not data:
request = [{
"path": "/rest/restconf/data/openconfig-vlan:vlans/",

This comment has been minimized.

Copy link
@ujwalkomarla

ujwalkomarla Sep 6, 2019

Contributor

Limit the restconf GET by specifying the depth parameter with value 5 : /rest/restconf/data/openconfig-vlan:vlans?depth=5


if want:
for w in want:
request_delete = deepcopy(self.VLAN_DELETE)

This comment has been minimized.

Copy link
@ujwalkomarla

ujwalkomarla Sep 6, 2019

Contributor

Move this line after 250.
You won't need to deepcopy unless you are actually creating a request.

if not have:
return requests
for h in have:
request_delete = deepcopy(self.VLAN_DELETE)

This comment has been minimized.

Copy link
@ujwalkomarla

ujwalkomarla Sep 6, 2019

Contributor

Move this line after 261.
You won't need to deepcopy unless you are actually creating a request.

'options': {
'name': {'type': 'str'},
'state': {
'choices': ['active', 'suspended'],

This comment has been minimized.

Copy link
@ujwalkomarla

ujwalkomarla Sep 6, 2019

Contributor

Choices are 'active' and 'suspend'.

if w.get('vlan_id'):
h = search_obj_in_list(w['vlan_id'], have, 'vlan_id')
if h:
continue

This comment has been minimized.

Copy link
@ujwalkomarla

ujwalkomarla Sep 6, 2019

Contributor

If the VLAN exists, you should check the name and state to make sure they are same as requested.

requests.append(request_post)

for h in have:
request_delete = deepcopy(self.VLAN_DELETE)

This comment has been minimized.

Copy link
@ujwalkomarla

ujwalkomarla Sep 6, 2019

Contributor

deepcopy only if required, move it inside the conditional.

request_body = self._update_vlan_config_body(w, request_body)
request_patch["data"]["openconfig-vlan:vlans"]["vlan"].append(request_body)
else:
request_post = deepcopy(self.VLAN_POST)

This comment has been minimized.

Copy link
@ujwalkomarla

ujwalkomarla Sep 6, 2019

Contributor

You have identical code on lines 159-163, 193-197, 228-232... Move it into a function.

@ujwalkomarla
Copy link
Contributor

left a comment

Otherwise LGTM

config["name"] = conf["name"]
if conf["status"] == "SUSPENDED":
config["state"] = "suspend"
else:

This comment has been minimized.

Copy link
@ujwalkomarla

ujwalkomarla Sep 7, 2019

Contributor

You can simplify with
`config["state"] = "suspend" if conf["status"] == "SUSPENDED" else conf["status"].lower()

collect the current configuration (as a dict from facts)
:rtype: A list
:returns: the commands necessary to migrate the current configuration

This comment has been minimized.

Copy link
@ujwalkomarla

ujwalkomarla Sep 7, 2019

Contributor

If you could replace 'commands' with 'requests' in the comments that would be good.

This comment has been minimized.

Copy link
@ujwalkomarla

ujwalkomarla Sep 7, 2019

Contributor

Thanks for the changes!
Looks good.

@ujwalkomarla
Copy link
Contributor

left a comment

shipit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.