Remove unnecessary if, and Fix Documentation (#34413)

* Remove unnecessary if, and Fix Documentation

Signed-off-by: Samer Deeb <samerd@mellanox.com>

* Fix aggregate spec for l2 interface

Signed-off-by: Samer Deeb <samerd@mellanox.com>

* Add aggregste test for l2 interface

Signed-off-by: Samer Deeb <samerd@mellanox.com>

* Fix MAC address case

Signed-off-by: Samer Deeb <samerd@mellanox.com>

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 <http://www.gnu.org/licenses/>.
-#
+# 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
 """
 

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):

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']]

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']]

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']]

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']]

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()

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']]

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)

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()

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']]

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)