From 5edb800b6d955379e30221c1901fe35b6cd58a80 Mon Sep 17 00:00:00 2001 From: Samer Deeb Date: Sun, 7 Jan 2018 00:35:50 -0800 Subject: [PATCH] Remove unnecessary if, and Fix Documentation (#34413) * Remove unnecessary if, and Fix Documentation Signed-off-by: Samer Deeb * Fix aggregate spec for l2 interface Signed-off-by: Samer Deeb * Add aggregste test for l2 interface Signed-off-by: Samer Deeb * Fix MAC address case Signed-off-by: Samer Deeb --- .../modules/network/mlnxos/mlnxos_command.py | 93 +++++++------------ .../modules/network/mlnxos/mlnxos_config.py | 34 +------ .../network/mlnxos/mlnxos_interface.py | 18 ++-- .../network/mlnxos/mlnxos_l2_interface.py | 25 +++-- .../network/mlnxos/mlnxos_l3_interface.py | 13 +-- .../modules/network/mlnxos/mlnxos_linkagg.py | 16 ++-- .../modules/network/mlnxos/mlnxos_lldp.py | 2 +- .../network/mlnxos/mlnxos_lldp_interface.py | 13 +-- .../modules/network/mlnxos/mlnxos_magp.py | 15 +-- .../modules/network/mlnxos/mlnxos_mlag_ipl.py | 12 +-- .../modules/network/mlnxos/mlnxos_vlan.py | 13 +-- .../mlnxos/test_mlnxos_l2_interface.py | 15 ++- 12 files changed, 107 insertions(+), 162 deletions(-) diff --git a/lib/ansible/modules/network/mlnxos/mlnxos_command.py b/lib/ansible/modules/network/mlnxos/mlnxos_command.py index 62faeff0aa..eb0227440a 100644 --- a/lib/ansible/modules/network/mlnxos/mlnxos_command.py +++ b/lib/ansible/modules/network/mlnxos/mlnxos_command.py @@ -1,27 +1,14 @@ #!/usr/bin/python # -# This file is part of Ansible -# -# Ansible is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# Ansible is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with Ansible. If not, see . -# +# Copyright: Ansible Project +# 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 ANSIBLE_METADATA = {'metadata_version': '1.1', 'status': ['preview'], - 'supported_by': 'network'} + 'supported_by': 'community'} DOCUMENTATION = """ --- @@ -31,65 +18,53 @@ version_added: "2.5" author: "Samer Deeb (@samerd)" short_description: Run commands on remote devices running Mellanox MLNX-OS description: - - >- - Sends arbitrary commands to an mlnxos node and returns the results - read from the device. This module includes an - argument that will cause the module to wait for a specific condition - before returning or timing out if the condition is not met. - - >- - This module does not support running commands in configuration mode. - Please use M(mlnxos_config) to configure Mellanox MLNX-OS devices. + - Sends arbitrary commands to an Mellanox MLNX-OS network device and returns + the results read from the device. This module includes an + argument that will cause the module to wait for a specific condition + before returning or timing out if the condition is not met. + - This module does not support running commands in configuration mode. + Please use M(mlnxos_config) to configure Mellanox MLNX-OS devices. notes: - - tested on Mellanox OS 3.6.4000 + - Tested on MLNX-OS 3.6.4000 options: commands: description: - - >- - List of commands to send to the remote mlnxos device over the - configured provider. The resulting output from the command - is returned. If the I(wait_for) argument is provided, the - module is not returned until the condition is satisfied or - the number of retries has expired. + - List of commands to send to the remote mlnxos device over the + configured provider. The resulting output from the command + is returned. If the I(wait_for) argument is provided, the + module is not returned until the condition is satisfied or + the number of retries has expired. required: true wait_for: description: - - >- - List of conditions to evaluate against the output of the - command. The task will wait for each condition to be true - before moving forward. If the conditional is not true - within the configured number of retries, the task fails. - See examples. - required: false - default: null + - List of conditions to evaluate against the output of the + command. The task will wait for each condition to be true + before moving forward. If the conditional is not true + within the configured number of retries, the task fails. + See examples. match: description: - - >- - The I(match) argument is used in conjunction with the - I(wait_for) argument to specify the match policy. Valid - values are C(all) or C(any). If the value is set to C(all) - then all conditionals in the wait_for must be satisfied. If - the value is set to C(any) then only one of the values must be - satisfied. - required: false + - The I(match) argument is used in conjunction with the + I(wait_for) argument to specify the match policy. Valid + values are C(all) or C(any). If the value is set to C(all) + then all conditionals in the wait_for must be satisfied. If + the value is set to C(any) then only one of the values must be + satisfied. default: all choices: ['any', 'all'] retries: description: - - >- - Specifies the number of retries a command should by tried - before it is considered failed. The command is run on the - target device every retry and evaluated against the - I(wait_for) conditions. - required: false + - Specifies the number of retries a command should by tried + before it is considered failed. The command is run on the + target device every retry and evaluated against the + I(wait_for) conditions. default: 10 interval: description: - - >- - Configures the interval in seconds to wait between retries - of the command. If the command does not pass the specified - conditions, the interval indicates how long to wait before - trying the command again. - required: false + - Configures the interval in seconds to wait between retries + of the command. If the command does not pass the specified + conditions, the interval indicates how long to wait before + trying the command again. default: 1 """ diff --git a/lib/ansible/modules/network/mlnxos/mlnxos_config.py b/lib/ansible/modules/network/mlnxos/mlnxos_config.py index 4177054598..8d5f184b21 100644 --- a/lib/ansible/modules/network/mlnxos/mlnxos_config.py +++ b/lib/ansible/modules/network/mlnxos/mlnxos_config.py @@ -20,7 +20,7 @@ short_description: Manage Mellanox MLNX-OS configuration sections description: - Mellanox MLNX-OS configurations uses a simple block indent file syntax for segmenting configuration into sections. This module provides - an implementation for working with MLNXOS configuration sections in + an implementation for working with MLNX-OS configuration sections in a deterministic way. options: lines: @@ -30,8 +30,6 @@ options: in the device running-config. Be sure to note the configuration command syntax as some commands are automatically modified by the device config parser. - required: false - default: null aliases: ['commands'] parents: description: @@ -39,8 +37,6 @@ options: the commands should be checked against. If the parents argument is omitted, the commands are checked against the set of top level or global commands. - required: false - default: null src: description: - Specifies the source path to the file that contains the configuration @@ -48,8 +44,6 @@ options: either be the full path on the Ansible control host or a relative path from the playbook or role root directory. This argument is mutually exclusive with I(lines), I(parents). - required: false - default: null before: description: - The ordered set of commands to push on to the command stack if @@ -57,16 +51,12 @@ options: the opportunity to perform configuration commands prior to pushing any changes without affecting how the set of commands are matched against the system. - required: false - default: null after: description: - The ordered set of commands to append to the end of the command stack if a change needs to be made. Just like with I(before) this allows the playbook designer to append a set of commands to be executed after the command set. - required: false - default: null match: description: - Instructs the module on the way to perform the matching of @@ -77,7 +67,6 @@ options: must be an equal match. Finally, if match is set to I(none), the module will not attempt to compare the source configuration with the running configuration on the remote device. - required: false default: line choices: ['line', 'strict', 'exact', 'none'] replace: @@ -88,7 +77,6 @@ options: mode. If the replace argument is set to I(block) then the entire command block is pushed to the device in configuration mode if any line is not correct - required: false default: line choices: ['line', 'block'] backup: @@ -98,7 +86,6 @@ options: changes are made. The backup file is written to the C(backup) folder in the playbook root directory. If the directory does not exist, it is created. - required: false default: no choices: ['yes', 'no'] config: @@ -107,35 +94,21 @@ options: the base configuration to be used to validate configuration changes necessary. If this argument is provided, the module will not download the running-config from the remote node. - required: false - default: null save: description: - The C(save) argument instructs the module to save the running- config to the startup-config at the conclusion of the module running. If check mode is specified, this argument is ignored. - required: false default: no choices: ['yes', 'no'] """ EXAMPLES = """ -# Note: examples below use the following provider dict to handle -# transport and authentication to the node. ---- -vars: - cli: - host: "{{ inventory_hostname }}" - username: admin - password: admin - authorize: yes - --- - mlnxos_config: lines: - snmp-server community - snmp-server host 10.2.2.2 traps version 2c - provider: "{{ cli }}" """ RETURN = """ @@ -154,8 +127,9 @@ backup_path: from ansible.module_utils.basic import AnsibleModule from ansible.module_utils.network.common.config import NetworkConfig, dumps -from ansible.module_utils.network.mlnxos.mlnxos import get_config, \ - load_config, run_commands +from ansible.module_utils.network.mlnxos.mlnxos import get_config +from ansible.module_utils.network.mlnxos.mlnxos import load_config +from ansible.module_utils.network.mlnxos.mlnxos import run_commands def get_candidate(module): diff --git a/lib/ansible/modules/network/mlnxos/mlnxos_interface.py b/lib/ansible/modules/network/mlnxos/mlnxos_interface.py index 6a5d1e8331..e6a6b22c22 100644 --- a/lib/ansible/modules/network/mlnxos/mlnxos_interface.py +++ b/lib/ansible/modules/network/mlnxos/mlnxos_interface.py @@ -62,7 +62,8 @@ options: description: - Purge Interfaces not defined in the aggregate parameter. This applies only for logical interface. - default: no + default: false + type: bool state: description: - State of the Interface configuration, C(up) means present and @@ -179,18 +180,15 @@ class MlnxosInterfaceModule(BaseMlnxosModule): return aggregate_spec def init_module(self): - """ main entry point for module execution + """ module initialization """ element_spec = self._get_element_spec() aggregate_spec = self._get_aggregate_spec(element_spec) - if aggregate_spec: - argument_spec = dict( - aggregate=dict(type='list', elements='dict', - options=aggregate_spec), - purge=dict(default=False, type='bool'), - ) - else: - argument_spec = dict() + argument_spec = dict( + aggregate=dict(type='list', elements='dict', + options=aggregate_spec), + purge=dict(default=False, type='bool'), + ) argument_spec.update(element_spec) required_one_of = [['name', 'aggregate']] mutually_exclusive = [['name', 'aggregate']] diff --git a/lib/ansible/modules/network/mlnxos/mlnxos_l2_interface.py b/lib/ansible/modules/network/mlnxos/mlnxos_l2_interface.py index 47463f9704..e22ad0d0b8 100644 --- a/lib/ansible/modules/network/mlnxos/mlnxos_l2_interface.py +++ b/lib/ansible/modules/network/mlnxos/mlnxos_l2_interface.py @@ -47,23 +47,23 @@ options: EXAMPLES = """ - name: configure Layer-2 interface mlnxos_l2_interface: - name: gigabitethernet0/0/1 + name: Eth1/1 mode: access access_vlan: 30 - name: remove Layer-2 interface configuration mlnxos_l2_interface: - name: gigabitethernet0/0/1 + name: Eth1/1 state: absent """ RETURN = """ commands: description: The list of configuration mode commands to send to the device - returned: always, except for the platforms that use Netconf transport to manage the device. + returned: always. type: list sample: - - interface gigabitethernet0/0/1 + - interface ethernet 1/1 - switchport mode access - switchport access vlan 30 """ @@ -74,8 +74,8 @@ from ansible.module_utils.basic import AnsibleModule from ansible.module_utils.six import iteritems from ansible.module_utils.network.common.utils import remove_default_spec -from ansible.module_utils.network.mlnxos.mlnxos import BaseMlnxosModule, \ - get_interfaces_config +from ansible.module_utils.network.mlnxos.mlnxos import BaseMlnxosModule +from ansible.module_utils.network.mlnxos.mlnxos import get_interfaces_config class MlnxosL2InterfaceModule(BaseMlnxosModule): @@ -96,7 +96,7 @@ class MlnxosL2InterfaceModule(BaseMlnxosModule): @classmethod def _get_aggregate_spec(cls, element_spec): aggregate_spec = deepcopy(element_spec) - aggregate_spec['vlan_id'] = dict(required=True) + aggregate_spec['name'] = dict(required=True) # remove default in aggregate spec, to handle common arguments remove_default_spec(aggregate_spec) @@ -107,13 +107,10 @@ class MlnxosL2InterfaceModule(BaseMlnxosModule): """ element_spec = self._get_element_spec() aggregate_spec = self._get_aggregate_spec(element_spec) - if aggregate_spec: - argument_spec = dict( - aggregate=dict(type='list', elements='dict', - options=aggregate_spec), - ) - else: - argument_spec = dict() + argument_spec = dict( + aggregate=dict(type='list', elements='dict', + options=aggregate_spec), + ) argument_spec.update(element_spec) required_one_of = [['name', 'aggregate']] mutually_exclusive = [['name', 'aggregate']] diff --git a/lib/ansible/modules/network/mlnxos/mlnxos_l3_interface.py b/lib/ansible/modules/network/mlnxos/mlnxos_l3_interface.py index 372b7caa43..c4e7456f8a 100644 --- a/lib/ansible/modules/network/mlnxos/mlnxos_l3_interface.py +++ b/lib/ansible/modules/network/mlnxos/mlnxos_l3_interface.py @@ -134,14 +134,11 @@ class MlnxosL3InterfaceModule(BaseMlnxosModule): """ element_spec = self._get_element_spec() aggregate_spec = self._get_aggregate_spec(element_spec) - if aggregate_spec: - argument_spec = dict( - aggregate=dict(type='list', elements='dict', - options=aggregate_spec), - purge=dict(default=False, type='bool'), - ) - else: - argument_spec = dict() + argument_spec = dict( + aggregate=dict(type='list', elements='dict', + options=aggregate_spec), + purge=dict(default=False, type='bool'), + ) argument_spec.update(element_spec) required_one_of = [['name', 'aggregate']] mutually_exclusive = [['name', 'aggregate']] diff --git a/lib/ansible/modules/network/mlnxos/mlnxos_linkagg.py b/lib/ansible/modules/network/mlnxos/mlnxos_linkagg.py index e7ed0a2aed..cdebb4481a 100644 --- a/lib/ansible/modules/network/mlnxos/mlnxos_linkagg.py +++ b/lib/ansible/modules/network/mlnxos/mlnxos_linkagg.py @@ -42,7 +42,8 @@ options: purge: description: - Purge link aggregation groups not defined in the I(aggregate) parameter. - default: no + default: false + type: bool state: description: - State of the link aggregation group. @@ -142,14 +143,11 @@ class MlnxosLinkAggModule(BaseMlnxosModule): """ element_spec = self._get_element_spec() aggregate_spec = self._get_aggregate_spec(element_spec) - if aggregate_spec: - argument_spec = dict( - aggregate=dict(type='list', elements='dict', - options=aggregate_spec), - purge=dict(default=False, type='bool'), - ) - else: - argument_spec = dict() + argument_spec = dict( + aggregate=dict(type='list', elements='dict', + options=aggregate_spec), + purge=dict(default=False, type='bool'), + ) argument_spec.update(element_spec) required_one_of = [['name', 'aggregate']] mutually_exclusive = [['name', 'aggregate']] diff --git a/lib/ansible/modules/network/mlnxos/mlnxos_lldp.py b/lib/ansible/modules/network/mlnxos/mlnxos_lldp.py index c4f0f645da..c9b757e610 100644 --- a/lib/ansible/modules/network/mlnxos/mlnxos_lldp.py +++ b/lib/ansible/modules/network/mlnxos/mlnxos_lldp.py @@ -63,7 +63,7 @@ class MlnxosLldpModule(BaseMlnxosModule): ) def init_module(self): - """ main entry point for module execution + """ module initialization """ element_spec = self._get_element_spec() argument_spec = dict() diff --git a/lib/ansible/modules/network/mlnxos/mlnxos_lldp_interface.py b/lib/ansible/modules/network/mlnxos/mlnxos_lldp_interface.py index a7aa16860d..f1faeb66b1 100644 --- a/lib/ansible/modules/network/mlnxos/mlnxos_lldp_interface.py +++ b/lib/ansible/modules/network/mlnxos/mlnxos_lldp_interface.py @@ -119,14 +119,11 @@ class MlnxosLldpInterfaceModule(BaseMlnxosModule): """ element_spec = self._get_element_spec() aggregate_spec = self._get_aggregate_spec(element_spec) - if aggregate_spec: - argument_spec = dict( - aggregate=dict(type='list', elements='dict', - options=aggregate_spec), - purge=dict(default=False, type='bool'), - ) - else: - argument_spec = dict() + argument_spec = dict( + aggregate=dict(type='list', elements='dict', + options=aggregate_spec), + purge=dict(default=False, type='bool'), + ) argument_spec.update(element_spec) required_one_of = [['name', 'aggregate']] mutually_exclusive = [['name', 'aggregate']] diff --git a/lib/ansible/modules/network/mlnxos/mlnxos_magp.py b/lib/ansible/modules/network/mlnxos/mlnxos_magp.py index 5079a66ddb..85eb898756 100644 --- a/lib/ansible/modules/network/mlnxos/mlnxos_magp.py +++ b/lib/ansible/modules/network/mlnxos/mlnxos_magp.py @@ -15,12 +15,12 @@ DOCUMENTATION = """ module: mlnxos_magp version_added: "2.5" author: "Samer Deeb (@samerd)" -short_description: Manage MAGP protocol on MLNX-OS network devices +short_description: Manage MAGP protocol on Mellanox MLNX-OS network devices description: - This module provides declarative management of MAGP protocol on vlan - interface of MLNX-OS network devices. + interface of Mellanox MLNX-OS network devices. notes: - - tested on Mellanox OS 3.6.4000 + - Tested on MLNX-OS 3.6.4000 options: magp_id: description: @@ -32,15 +32,15 @@ options: required: true state: description: - - vlan interface state + - MAGP state. default: present choices: ['present', 'absent', 'enabled', 'disabled'] router_ip: description: - - MAGP router ip address + - MAGP router IP address. router_mac: description: - - MAGP router MAC address + - MAGP router MAC address. """ EXAMPLES = """ @@ -188,7 +188,10 @@ class MlnxosMagpModule(BaseMlnxosModule): self._commands.append(cmd) req_router_mac = self._required_config['router_mac'] curr_router_mac = curr_magp_data.get('router_mac') + if curr_router_mac: + curr_router_mac = curr_router_mac.lower() if req_router_mac: + req_router_mac = req_router_mac.lower() if curr_router_mac != req_router_mac or create_new_magp: cmd = '%s ip virtual-router mac-address %s' % ( magp_prefix, req_router_mac) diff --git a/lib/ansible/modules/network/mlnxos/mlnxos_mlag_ipl.py b/lib/ansible/modules/network/mlnxos/mlnxos_mlag_ipl.py index 333708cdea..2ed7259095 100644 --- a/lib/ansible/modules/network/mlnxos/mlnxos_mlag_ipl.py +++ b/lib/ansible/modules/network/mlnxos/mlnxos_mlag_ipl.py @@ -15,12 +15,12 @@ DOCUMENTATION = """ module: mlnxos_mlag_ipl version_added: "2.5" author: "Samer Deeb (@samerd)" -short_description: Manage IPL (inter-peer link) on MLNX-OS network devices +short_description: Manage IPL (inter-peer link) on Mellanox MLNX-OS network devices description: - This module provides declarative management of IPL (inter-peer link) - management on MLNX-OS network devices. + management on Mellanox MLNX-OS network devices. notes: - - tested on Mellanox OS 3.6.4000 + - Tested on MLNX-OS 3.6.4000 options: name: description: @@ -31,12 +31,12 @@ options: - Name of the IPL vlan interface. state: description: - - IPL state + - IPL state. default: present choices: ['present', 'absent'] peer_address: description: - - IPL peer IP address + - IPL peer IP address. """ EXAMPLES = """ @@ -84,7 +84,7 @@ class MlnxosMlagIplModule(BaseMlnxosModule): ) def init_module(self): - """ main entry point for module execution + """ module initialization """ element_spec = self._get_element_spec() argument_spec = dict() diff --git a/lib/ansible/modules/network/mlnxos/mlnxos_vlan.py b/lib/ansible/modules/network/mlnxos/mlnxos_vlan.py index fb68baff0a..1c16f8da9a 100644 --- a/lib/ansible/modules/network/mlnxos/mlnxos_vlan.py +++ b/lib/ansible/modules/network/mlnxos/mlnxos_vlan.py @@ -96,14 +96,11 @@ class MlnxosVlanModule(BaseMlnxosModule): """ element_spec = self._get_element_spec() aggregate_spec = self._get_aggregate_spec(element_spec) - if aggregate_spec: - argument_spec = dict( - aggregate=dict(type='list', elements='dict', - options=aggregate_spec), - purge=dict(default=False, type='bool'), - ) - else: - argument_spec = dict() + argument_spec = dict( + aggregate=dict(type='list', elements='dict', + options=aggregate_spec), + purge=dict(default=False, type='bool'), + ) argument_spec.update(element_spec) required_one_of = [['vlan_id', 'aggregate']] mutually_exclusive = [['vlan_id', 'aggregate']] diff --git a/test/units/modules/network/mlnxos/test_mlnxos_l2_interface.py b/test/units/modules/network/mlnxos/test_mlnxos_l2_interface.py index 1f99346101..2a926daa8c 100644 --- a/test/units/modules/network/mlnxos/test_mlnxos_l2_interface.py +++ b/test/units/modules/network/mlnxos/test_mlnxos_l2_interface.py @@ -20,11 +20,8 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type -import json - from ansible.compat.tests.mock import patch from ansible.modules.network.mlnxos import mlnxos_l2_interface -from ansible.module_utils.network.mlnxos import mlnxos as mlnxos_utils from units.modules.utils import set_module_args from .mlnxos_module import TestMlnxosModule, load_fixture @@ -103,3 +100,15 @@ class TestMlnxosInterfaceModule(TestMlnxosModule): 'switchport access vlan 10', 'switchport hybrid allowed-vlan add 11', 'exit'] self.execute_module(changed=True, commands=commands) + + def test_aggregate(self): + aggregate = list() + aggregate.append(dict(name='Eth1/10')) + aggregate.append(dict(name='Eth1/12')) + + set_module_args(dict(aggregate=aggregate, access_vlan=10)) + commands = ['interface ethernet 1/10', 'switchport mode access', + 'switchport access vlan 10', 'exit', + 'interface ethernet 1/12', 'switchport mode access', + 'switchport access vlan 10', 'exit'] + self.execute_module(changed=True, commands=commands, sort=False)