Deprecate nxos_portchannel and add nxos_linkagg DI module (#33376)

* deprecate nxos_portchannel

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

* Add nxos_linkagg DI module

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

* remove nxos_portchannel unit test

* fix syntax issues

* fix ansible-doc failure

* update nxos_portchannel unittest

* minor fixes and integration test

* update nxos_linkagg

CHANGELOG.md

@@ -33,6 +33,7 @@ Ansible Changes By Release
   simply remove check_invalid_arguments if they set it to the default of True.
   check_invalid_arguments will be removed in Ansible-2.9.
 * nxos_ip_interface module is deprecated in Ansible 2.5. Use nxos_l3_interface module instead.
+* nxos_portchannel module is deprecated in Ansible 2.5. Use nxos_linkagg module instead.
 
 ### Minor Changes
 * added a few new magic vars corresponding to configuration/command line options:

docs/docsite/rst/porting_guide_2.5.rst

@@ -80,6 +80,7 @@ The following modules will be removed in Ansible 2.9. Please update update your
 
 * :ref:`fixme <fixme>`
 * :ref:`nxos_ip_interface <nxos_ip_interface>` use :ref:`nxos_l3_interface <nxos_l3_interface>` instead.
+* :ref:`nxos_portchannel <nxos_portchannel>` use :ref:`nxos_linkagg <nxos_linkagg>` instead.
 
 Noteworthy module changes
 -------------------------

lib/ansible/modules/network/nxos/_nxos_portchannel.py

@@ -17,7 +17,7 @@
 #
 
 ANSIBLE_METADATA = {'metadata_version': '1.1',
-                    'status': ['preview'],
+                    'status': ['deprecated'],
                     'supported_by': 'network'}
 
 DOCUMENTATION = '''
@@ -25,6 +25,7 @@ DOCUMENTATION = '''
 module: nxos_portchannel
 extends_documentation_fragment: nxos
 version_added: "2.2"
+deprecated: Deprecated in 2.5. Use M(nxos_linkagg) instead.
 short_description: Manages port-channel interfaces.
 description:
   - Manages port-channel specific configuration parameters.

lib/ansible/modules/network/nxos/nxos_linkagg.py

@@ -0,0 +1,407 @@
+#!/usr/bin/python
+# -*- coding: utf-8 -*-
+
+# (c) 2017, Ansible by Red Hat, inc
+# 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'}
+
+DOCUMENTATION = """
+---
+module: nxos_linkagg
+extends_documentation_fragment: nxos
+version_added: "2.5"
+short_description: Manage link aggregation groups on Cisco NXOS devices.
+description:
+  - This module provides declarative management of link aggregation groups
+    on Cisco NXOS devices.
+author:
+  - Trishna Guha (@trishnaguha)
+notes:
+  - Tested against NXOSv 7.0(3)I5(1).
+  - C(state=absent) removes the portchannel config and interface if it
+    already exists. If members to be removed are not explicitly
+    passed, all existing members (if any), are removed.
+  - Members must be a list.
+  - LACP needs to be enabled first if active/passive modes are used.
+options:
+  group:
+    description:
+      - Channel-group number for the port-channel
+        Link aggregation group.
+    required: true
+  mode:
+    description:
+      - Mode for the link aggregation group.
+    required: false
+    default: on
+    choices: ['active','passive','on']
+  min_links:
+    description:
+      - Minimum number of ports required up
+        before bringing up the link aggregation group.
+    required: false
+    default: null
+  members:
+    description:
+      - List of interfaces that will be managed in the link aggregation group.
+    required: false
+    default: null
+  force:
+    description:
+      - When true it forces link aggregation group members to match what
+        is declared in the members param. This can be used to remove members.
+    required: false
+    choices: [True, False]
+    default: False
+  aggregate:
+    description: List of link aggregation definitions.
+  state:
+    description:
+      - State of the link aggregation group.
+    required: false
+    default: present
+    choices: ['present','absent']
+"""
+
+EXAMPLES = """
+- name: create link aggregation group
+  nxos_linkagg:
+    group: 99
+    state: present
+
+- name: delete link aggregation group
+  nxos_linkagg:
+    group: 99
+    state: absent
+
+- name: set link aggregation group to members
+  nxos_linkagg:
+    group: 10
+    min_links: 3
+    mode: active
+    members:
+      - Ethernet1/2
+      - Ethernet1/4
+
+- name: remove link aggregation group from Ethernet1/2
+  nxos_linkagg:
+    group: 10
+    min_links: 3
+    mode: active
+    members:
+      - Ethernet1/4
+
+- name: Create aggregate of linkagg definitions
+  nxos_linkagg:
+    aggregate:
+      - { group: 3 }
+      - { group: 100, min_links: 3 }
+
+- name: Remove aggregate of linkagg definitions
+  nxos_linkagg:
+    aggregate:
+      - { group: 3 }
+      - { group: 100, min_links: 3 }
+    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.
+  type: list
+  sample:
+    - interface port-channel 30
+    - lacp min-links 5
+    - interface Ethernet2/1
+    - channel-group 30 mode active
+    - no interface port-channel 30
+"""
+
+import re
+from copy import deepcopy
+
+from ansible.module_utils.network.nxos.nxos import get_config, load_config, run_commands
+from ansible.module_utils.network.nxos.nxos import nxos_argument_spec
+from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.network.common.utils import remove_default_spec
+
+
+def execute_show_command(command, module):
+    provider = module.params['provider']
+    if provider['transport'] == 'cli':
+        if 'show port-channel summary' in command:
+            command += ' | json'
+        cmds = [command]
+        body = run_commands(module, cmds)
+    elif provider['transport'] == 'nxapi':
+        cmds = [command]
+        body = run_commands(module, cmds)
+
+    return body
+
+
+def search_obj_in_list(group, lst):
+    for o in lst:
+        if o['group'] == group:
+            return o
+
+
+def map_obj_to_commands(updates, module):
+    commands = list()
+    want, have = updates
+    purge = module.params['purge']
+    force = module.params['force']
+
+    for w in want:
+        group = w['group']
+        mode = w['mode']
+        min_links = w['min_links']
+        members = w.get('members') or []
+        state = w['state']
+        del w['state']
+
+        obj_in_have = search_obj_in_list(group, have)
+
+        if state == 'absent':
+            if obj_in_have:
+                members_to_remove = list(set(obj_in_have['members']) - set(members))
+                if members_to_remove:
+                    for m in members_to_remove:
+                        commands.append('interface {0}'.format(m))
+                        commands.append('no channel-group {0}'.format(obj_in_have['group']))
+                        commands.append('exit')
+                commands.append('no interface port-channel {0}'.format(group))
+
+        elif state == 'present':
+            if not obj_in_have:
+                commands.append('interface port-channel {0}'.format(group))
+                if min_links != 'None':
+                    commands.append('lacp min-links {0}'.format(min_links))
+                commands.append('exit')
+                if members:
+                    for m in members:
+                        commands.append('interface {0}'.format(m))
+                        if force:
+                            commands.append('channel-group {0} force mode {1}'.format(group, mode))
+                        else:
+                            commands.append('channel-group {0} mode {1}'.format(group, mode))
+
+            else:
+                if members:
+                    if not obj_in_have['members']:
+                        for m in members:
+                            commands.append('interface port-channel {0}'.format(group))
+                            commands.append('exit')
+                            commands.append('interface {0}'.format(m))
+                            if force:
+                                commands.append('channel-group {0} force mode {1}'.format(group, mode))
+                            else:
+                                commands.append('channel-group {0} mode {1}'.format(group, mode))
+
+                    elif set(members) != set(obj_in_have['members']):
+                        missing_members = list(set(members) - set(obj_in_have['members']))
+                        for m in missing_members:
+                            commands.append('interface port-channel {0}'.format(group))
+                            commands.append('exit')
+                            commands.append('interface {0}'.format(m))
+                            if force:
+                                commands.append('channel-group {0} force mode {1}'.format(group, mode))
+                            else:
+                                commands.append('channel-group {0} mode {1}'.format(group, mode))
+
+                        superfluous_members = list(set(obj_in_have['members']) - set(members))
+                        for m in superfluous_members:
+                            commands.append('interface port-channel {0}'.format(group))
+                            commands.append('exit')
+                            commands.append('interface {0}'.format(m))
+                            commands.append('no channel-group {0}'.format(group))
+    if purge:
+        for h in have:
+            obj_in_want = search_obj_in_list(h['group'], want)
+            if not obj_in_want:
+                commands.append('no interface port-channel {0}'.format(h['group']))
+
+    return commands
+
+
+def map_params_to_obj(module):
+    obj = []
+    aggregate = module.params.get('aggregate')
+    if aggregate:
+        for item in aggregate:
+            for key in item:
+                if item.get(key) is None:
+                    item[key] = module.params[key]
+
+            d = item.copy()
+            d['group'] = str(d['group'])
+            d['min_links'] = str(d['min_links'])
+
+            obj.append(d)
+    else:
+        obj.append({
+            'group': str(module.params['group']),
+            'mode': module.params['mode'],
+            'min_links': str(module.params['min_links']),
+            'members': module.params['members'],
+            'state': module.params['state']
+        })
+
+    return obj
+
+
+def parse_min_links(module, group):
+    min_links = None
+
+    flags = ['| section interface.port-channel{0}'.format(group)]
+    config = get_config(module, flags=flags)
+    match = re.search(r'lacp min-links (\S+)', config, re.M)
+    if match:
+        min_links = match.group(1)
+
+    return min_links
+
+
+def parse_mode(module, m):
+    mode = None
+
+    flags = ['| section interface.{0}'.format(m)]
+    config = get_config(module, flags=flags)
+    match = re.search(r'mode (\S+)', config, re.M)
+    if match:
+        mode = match.group(1)
+
+    return mode
+
+
+def get_members(channel):
+    members = []
+    if 'TABLE_member' in channel.keys():
+        interfaces = channel['TABLE_member']['ROW_member']
+    else:
+        return list()
+
+    if isinstance(interfaces, dict):
+        members.append(interfaces.get('port'))
+    elif isinstance(interfaces, list):
+        for i in interfaces:
+            members.append(i.get('port'))
+
+    return members
+
+
+def parse_members(output, group):
+    channels = output['TABLE_channel']['ROW_channel']
+
+    if isinstance(channels, list):
+        for channel in channels:
+            if channel['group'] == group:
+                members = get_members(channel)
+    elif isinstance(channels, dict):
+        if channels['group'] == group:
+            members = get_members(channels)
+    else:
+        return list()
+
+    return members
+
+
+def parse_channel_options(module, output, channel):
+    obj = {}
+
+    group = channel['group']
+    obj['group'] = group
+    obj['min-links'] = parse_min_links(module, group)
+    members = parse_members(output, group)
+    obj['members'] = members
+    for m in members:
+        obj['mode'] = parse_mode(module, m)
+
+    return obj
+
+
+def map_config_to_obj(module):
+    objs = list()
+    output = execute_show_command('show port-channel summary', module)[0]
+    if not output:
+        return list()
+
+    try:
+        channels = output['TABLE_channel']['ROW_channel']
+    except KeyError:
+        return objs
+
+    if channels:
+        if isinstance(channels, list):
+            for channel in channels:
+                obj = parse_channel_options(module, output, channel)
+                objs.append(obj)
+
+        elif isinstance(channels, dict):
+            obj = parse_channel_options(module, output, channels)
+            objs.append(obj)
+
+    return objs
+
+
+def main():
+    """ main entry point for module execution
+    """
+    element_spec = dict(
+        group=dict(type='int'),
+        mode=dict(required=False, choices=['on', 'active', 'passive'], default='on', type='str'),
+        min_links=dict(required=False, default=None, type='int'),
+        members=dict(required=False, default=None, type='list'),
+        force=dict(required=False, default=False, type='bool'),
+        state=dict(required=False, choices=['absent', 'present'], default='present')
+    )
+
+    aggregate_spec = deepcopy(element_spec)
+    aggregate_spec['group'] = dict(required=True)
+
+    # remove default in aggregate spec, to handle common arguments
+    remove_default_spec(aggregate_spec)
+
+    argument_spec = dict(
+        aggregate=dict(type='list', elements='dict', options=aggregate_spec),
+        purge=dict(default=False, type='bool')
+    )
+
+    argument_spec.update(element_spec)
+    argument_spec.update(nxos_argument_spec)
+
+    required_one_of = [['group', 'aggregate']]
+    mutually_exclusive = [['group', 'aggregate']]
+    module = AnsibleModule(argument_spec=argument_spec,
+                           required_one_of=required_one_of,
+                           mutually_exclusive=mutually_exclusive,
+                           supports_check_mode=True)
+
+    warnings = list()
+    result = {'changed': False}
+    if warnings:
+        result['warnings'] = warnings
+
+    want = map_params_to_obj(module)
+    have = map_config_to_obj(module)
+
+    commands = map_obj_to_commands((want, have), module)
+    result['commands'] = commands
+
+    if commands:
+        if not module.check_mode:
+            load_config(module, commands)
+        result['changed'] = True
+
+    module.exit_json(**result)
+
+if __name__ == '__main__':
+    main()

test/integration/nxos.yaml

@@ -563,6 +563,15 @@
             failed_modules: "{{ failed_modules }} + [ 'nxos_l3_interface' ]"
             test_failed: true
 
+    - block:
+      - include_role:
+          name: nxos_linkagg
+        when: "limit_to in ['*', 'nxos_linkagg']"
+      rescue:
+        - set_fact:
+            failed_modules: "{{ failed_modules }} + [ 'nxos_linkagg' ]"
+            test_failed: true
+
 ###########
     - debug: var=failed_modules
       when: test_failed

test/integration/targets/nxos_linkagg/defaults/main.yaml

@@ -0,0 +1,3 @@
+---
+testcase: "*"
+test_items: []

test/integration/targets/nxos_linkagg/meta/main.yml

@@ -0,0 +1,2 @@
+dependencies:
+  - prepare_nxos_tests

test/integration/targets/nxos_linkagg/tasks/cli.yaml

@@ -0,0 +1,25 @@
+---
+- name: collect common cli test cases
+  find:
+    paths: "{{ role_path }}/tests/common"
+    patterns: "{{ testcase }}.yaml"
+  register: test_cases
+
+- name: collect cli test cases
+  find:
+    paths: "{{ role_path }}/tests/cli"
+    patterns: "{{ testcase }}.yaml"
+  register: cli_cases
+
+- set_fact:
+    test_cases:
+      files: "{{ test_cases.files }} + {{ cli_cases.files }}"
+
+- name: set test_items
+  set_fact: test_items="{{ test_cases.files | map(attribute='path') | list }}"
+
+- name: run test case
+  include: "{{ test_case_to_run }} connection={{ cli }}"
+  with_items: "{{ test_items }}"
+  loop_control:
+    loop_var: test_case_to_run

test/integration/targets/nxos_linkagg/tasks/main.yaml

@@ -0,0 +1,3 @@
+---
+- { include: cli.yaml, tags: ['cli'] }
+- { include: nxapi.yaml, tags: ['nxapi'] }

test/integration/targets/nxos_linkagg/tasks/nxapi.yaml

@@ -0,0 +1,25 @@
+---
+- name: collect common nxapi test cases
+  find:
+    paths: "{{ role_path }}/tests/common"
+    patterns: "{{ testcase }}.yaml"
+  register: test_cases
+
+- name: collect nxapi test cases
+  find:
+    paths: "{{ role_path }}/tests/nxapi"
+    patterns: "{{ testcase }}.yaml"
+  register: nxapi_cases
+
+- set_fact:
+    test_cases:
+      files: "{{ test_cases.files }} + {{ nxapi_cases.files }}"
+
+- name: set test_items
+  set_fact: test_items="{{ test_cases.files | map(attribute='path') | list }}"
+
+- name: run test case
+  include: "{{ test_case_to_run }} connection={{ nxapi }}"
+  with_items: "{{ test_items }}"
+  loop_control:
+    loop_var: test_case_to_run

test/integration/targets/nxos_linkagg/tests/common/sanity.yaml

@@ -0,0 +1,193 @@
+---
+- debug: msg="START TRANSPORT:{{ connection.transport }} nxos_linkagg sanity test"
+
+- set_fact: testint1="{{ nxos_int1 }}"
+- set_fact: testint2="{{ nxos_int2 }}"
+
+- name: "Enable feature LACP"
+  nxos_feature:
+    feature: lacp
+    state: enabled
+    provider: "{{ connection }}"
+  ignore_errors: yes
+
+- name: setup - remove config used in test(part1)
+  nxos_config:
+    lines:
+      - no interface port-channel 20
+      - no interface port-channel 100
+    provider: "{{ connection }}"
+
+- name: setup - remove config used in test(part2)
+  nxos_config:
+    lines:
+      - no channel-group 20
+    provider: "{{ connection }}"
+    parents: "{{ item }}"
+  loop:
+    - "interface {{ testint1 }}"
+    - "interface {{ testint2 }}"
+
+- name: create linkagg
+  nxos_linkagg: &create
+    group: 20
+    state: present
+    provider: "{{ connection }}"
+  register: result
+
+- assert:
+    that:
+      - 'result.changed == true'
+      - '"interface port-channel 20" in result.commands'
+
+- name: create linkagg(Idempotence)
+  nxos_linkagg: *create
+  register: result
+
+- assert:
+    that:
+      - 'result.changed == false'
+
+- name: set link aggregation group to members declaratively
+  nxos_linkagg: &configure_member
+    group: 20
+    mode: active
+    force: True
+    members:
+      - "{{ testint1 }}"
+      - "{{ testint2 }}"
+    provider: "{{ connection }}"
+  register: result
+
+- assert:
+    that:
+      - 'result.changed == true'
+      - '"interface {{ testint1 }}" in result.commands'
+      - '"channel-group 20 force mode active" in result.commands'
+      - '"interface {{ testint2 }}" in result.commands'
+      - '"channel-group 20 force mode active" in result.commands'
+
+- name: set link aggregation group to members(Idempotence)
+  nxos_linkagg: *configure_member
+  register: result
+
+- assert:
+    that:
+      - 'result.changed == false'
+
+- name: remove link aggregation group from member declaratively
+  nxos_linkagg: &remove_member
+    group: 20
+    mode: active
+    force: True
+    members:
+      - "{{ testint2 }}"
+    provider: "{{ connection }}"
+  register: result
+
+- assert:
+    that:
+      - 'result.changed == true'
+      - '"interface {{ testint1 }}" in result.commands'
+      - '"no channel-group 20" in result.commands'
+
+- name: remove link aggregation group from member(Idempotence)
+  nxos_linkagg: *remove_member
+  register: result
+
+- assert:
+    that:
+      - 'result.changed == false'
+
+- name: remove linkagg
+  nxos_linkagg: &remove
+    group: 20
+    state: absent
+    provider: "{{ connection }}"
+  register: result
+
+- assert:
+    that:
+      - 'result.changed == true'
+      - '"no interface port-channel 20" in result.commands'
+
+- name: remove linkagg(Idempotence)
+  nxos_linkagg: *remove
+  register: result
+
+- assert:
+    that:
+      - 'result.changed == false'
+
+- name: create aggregate of linkagg definitions
+  nxos_linkagg: &create_agg
+    aggregate:
+      - { group: 20, min_links: 3 }
+      - { group: 100, min_links: 4 }
+    provider: "{{ connection }}"
+  register: result
+
+- assert:
+    that:
+      - 'result.changed == true'
+      - '"interface port-channel 20" in result.commands'
+      - '"lacp min-links 3" in result.commands'
+      - '"interface port-channel 100" in result.commands'
+      - '"lacp min-links 4" in result.commands'
+
+- name: create aggregate of linkagg definitions(Idempotence)
+  nxos_linkagg: *create_agg
+  register: result
+
+- assert:
+    that:
+      - 'result.changed == false'
+
+- name: remove aggregate of linkagg definitions
+  nxos_linkagg: &remove_agg
+    aggregate:
+      - { group: 20, min_links: 3 }
+      - { group: 100, min_links: 4 }
+    provider: "{{ connection }}"
+    state: absent
+  register: result
+
+- assert:
+    that:
+      - 'result.changed == true'
+      - '"no interface port-channel 20" in result.commands'
+      - '"no interface port-channel 100" in result.commands'
+
+- name: remove aggregate of linkagg definitions(Idempotence)
+  nxos_linkagg: *remove_agg
+  register: result
+
+- assert:
+    that:
+      - 'result.changed == false'
+ 
+- name: teardown - remove config used in test(part1)
+  nxos_config:
+    lines:
+      - no interface port-channel 20
+      - no interface port-channel 100
+    provider: "{{ connection }}"
+
+- name: teardown - remove config used in test(part2)
+  nxos_config:
+    lines:
+      - no channel-group 20
+    provider: "{{ connection }}"
+    parents: "{{ item }}"
+  loop:
+    - "interface {{ testint1 }}"
+    - "interface {{ testint2 }}"
+
+- name: "Disable feature LACP"
+  nxos_feature:
+    feature: lacp
+    state: disabled
+    timeout: 60
+    provider: "{{ connection }}"
+
+- debug: msg="END TRANSPORT:{{ connection.transport }} nxos_linkagg sanity test"

test/units/modules/network/nxos/test_nxos_portchannel.py

@@ -20,27 +20,27 @@ from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
 from ansible.compat.tests.mock import patch
-from ansible.modules.network.nxos import nxos_portchannel
+from ansible.modules.network.nxos import _nxos_portchannel
 from .nxos_module import TestNxosModule, set_module_args
 
 
 class TestNxosPortchannelModule(TestNxosModule):
 
-    module = nxos_portchannel
+    module = _nxos_portchannel
 
     def setUp(self):
         super(TestNxosPortchannelModule, self).setUp()
 
-        self.mock_run_commands = patch('ansible.modules.network.nxos.nxos_portchannel.run_commands')
+        self.mock_run_commands = patch('ansible.modules.network.nxos._nxos_portchannel.run_commands')
         self.run_commands = self.mock_run_commands.start()
 
-        self.mock_load_config = patch('ansible.modules.network.nxos.nxos_portchannel.load_config')
+        self.mock_load_config = patch('ansible.modules.network.nxos._nxos_portchannel.load_config')
         self.load_config = self.mock_load_config.start()
 
-        self.mock_get_config = patch('ansible.modules.network.nxos.nxos_portchannel.get_config')
+        self.mock_get_config = patch('ansible.modules.network.nxos._nxos_portchannel.get_config')
         self.get_config = self.mock_get_config.start()
 
-        self.mock_get_capabilities = patch('ansible.modules.network.nxos.nxos_portchannel.get_capabilities')
+        self.mock_get_capabilities = patch('ansible.modules.network.nxos._nxos_portchannel.get_capabilities')
         self.get_capabilities = self.mock_get_capabilities.start()
         self.get_capabilities.return_value = {'network_api': 'cliconf'}