OSC integration - port extensions

Add port mac-learning & fix port provider-security-group to the
openstack client.

Change-Id: Ie3cefd1286d0f9dcfac2fe623ddb17b90e3f58bf

vmware_nsx/osc/v2/port.py

@@ -16,22 +16,27 @@
 """Port action implementations with nsx extensions"""
 
 from openstackclient.network.v2 import port
+from osc_lib import utils as osc_utils
 
 from vmware_nsx._i18n import _
 from vmware_nsx.osc.v2 import utils
 
 
-def add_nsx_extensions_to_parser(parser, client_manager):
+def add_nsx_extensions_to_parser(parser, client_manager, is_create=True):
-    # Provider security group
+    allowed_extensions = utils.get_extensions(client_manager)
-    if 'provider-security-group' in utils.get_extensions(client_manager):
+    # Provider security group (only for create action)
+    if (is_create and
+        'provider-security-group' in allowed_extensions):
         parser.add_argument(
-            '--provider-security-groups',
+            '--provider-security-group',
-            metavar='<provider-security-groups>',
+            metavar='<provider-security-group>',
             action='append',
             dest='provider_security_groups',
-            help=_("Provider security groups")
+            help=_("Provider Security group to associate with this port "
+                   "(name or ID) "
+                   "(repeat option to set multiple security groups)")
         )
-    if 'vnic-index' in utils.get_extensions(client_manager):
+    if 'vnic-index' in allowed_extensions:
         # vnic index
         parser.add_argument(
             '--vnic-index',
@@ -39,6 +44,19 @@ def add_nsx_extensions_to_parser(parser, client_manager):
             metavar='<vnic-index>',
             help=_("Vnic index")
         )
+    if 'mac-learning' in allowed_extensions:
+        # mac-learning-enabled
+        mac_learning_group = parser.add_mutually_exclusive_group()
+        mac_learning_group.add_argument(
+            '--enable-mac-learning',
+            action='store_true',
+            help=_("Enable MAC learning")
+        )
+        mac_learning_group.add_argument(
+            '--disable-mac-learning',
+            action='store_true',
+            help=_("Disable MAC learning (Default")
+        )
 
 
 # overriding the port module global method, to add the nsx extensions
@@ -46,18 +64,28 @@ super_get_attrs = port._get_attrs
 
 
 def _get_plugin_attrs(client_manager, parsed_args):
+    allowed_extensions = utils.get_extensions(client_manager)
     attrs = super_get_attrs(client_manager, parsed_args)
     # Provider security groups
-    if 'provider-security-group' in utils.get_extensions(client_manager):
+    if 'provider-security-group' in allowed_extensions:
-        if parsed_args.provider_security_groups is not None:
+        if (hasattr(parsed_args, 'provider_security_groups') and
-            attrs['provider_security_groups'] = (
+            parsed_args.provider_security_groups is not None):
-                parsed_args.provider_security_groups)
+            attrs['provider_security_groups'] = [
-            parsed_args.provider_security_groups = None
+                client_manager.network.find_security_group(
-    if 'vnic-index' in utils.get_extensions(client_manager):
+                    sg, ignore_missing=False).id
+                for sg in parsed_args.provider_security_groups]
+
+    if 'vnic-index' in allowed_extensions:
         # Vnic index
         if parsed_args.vnic_index is not None:
             attrs['vnic_index'] = parsed_args.vnic_index
             parsed_args.vnic_index = None
+    if 'mac-learning' in allowed_extensions:
+        # mac-learning-enabled
+        if parsed_args.enable_mac_learning:
+            attrs['mac_learning_enabled'] = True
+        if parsed_args.disable_mac_learning:
+            attrs['mac_learning_enabled'] = False
 
     return attrs
 
@@ -65,13 +93,19 @@ def _get_plugin_attrs(client_manager, parsed_args):
 port._get_attrs = _get_plugin_attrs
 
 
+# Update the port module global _formatters, to format provider security
+# groups too
+port._formatters['provider_security_groups'] = osc_utils.format_list
+
+
 class NsxCreatePort(port.CreatePort):
     """Create a new port with vmware nsx extensions """
 
     def get_parser(self, prog_name):
         # Add the nsx attributes to the neutron port attributes
         parser = super(NsxCreatePort, self).get_parser(prog_name)
-        add_nsx_extensions_to_parser(parser, self.app.client_manager)
+        add_nsx_extensions_to_parser(parser, self.app.client_manager,
+                                     is_create=True)
         return parser
 
 
@@ -81,5 +115,6 @@ class NsxSetPort(port.SetPort):
     def get_parser(self, prog_name):
         # Add the nsx attributes to the neutron port attributes
         parser = super(NsxSetPort, self).get_parser(prog_name)
-        add_nsx_extensions_to_parser(parser, self.app.client_manager)
+        add_nsx_extensions_to_parser(parser, self.app.client_manager,
+                                     is_create=False)
         return parser

vmware_nsx/tests/unit/osc/v2/test_port.py

@@ -16,13 +16,16 @@
 import mock
 import re
 
+from openstackclient.tests.unit.network.v2 import fakes as network_fakes
 from openstackclient.tests.unit.network.v2 import test_port
 from openstackclient.tests.unit import utils as tests_utils
 
 from vmware_nsx.osc.v2 import port
 
 
-supported_extensions = ('vnic-index', 'provider-security-group')
+supported_extensions = ('vnic-index',
+                        'provider-security-group',
+                        'mac-learning')
 
 
 class TestCreatePort(test_port.TestCreatePort):
@@ -78,6 +81,136 @@ class TestCreatePort(test_port.TestCreatePort):
     def test_create_with_illegal_vnic_index(self):
         self._test_create_with_vnix_index('illegal', is_valid=False)
 
+    def test_create_with_provider_security_group(self):
+        # create a port with 1 provider security group
+        secgroup = network_fakes.FakeSecurityGroup.create_one_security_group()
+        self.network.find_security_group = mock.Mock(return_value=secgroup)
+        arglist = [
+            '--network', self._port.network_id,
+            '--provider-security-group', secgroup.id,
+            'test-port',
+        ]
+        verifylist = [
+            ('network', self._port.network_id,),
+            ('enable', True),
+            ('provider_security_groups', [secgroup.id]),
+            ('name', 'test-port'),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        columns, data = (self.cmd.take_action(parsed_args))
+
+        self.network.create_port.assert_called_once_with(**{
+            'admin_state_up': True,
+            'network_id': self._port.network_id,
+            'provider_security_groups': [secgroup.id],
+            'name': 'test-port',
+        })
+
+        ref_columns, ref_data = self._get_common_cols_data(self._port)
+        self.assertEqual(ref_columns, columns)
+        self.assertEqual(ref_data, data)
+
+    def test_create_with_provider_security_groups(self):
+        # create a port with few provider security groups
+        sg_1 = network_fakes.FakeSecurityGroup.create_one_security_group()
+        sg_2 = network_fakes.FakeSecurityGroup.create_one_security_group()
+        self.network.find_security_group = mock.Mock(side_effect=[sg_1, sg_2])
+        arglist = [
+            '--network', self._port.network_id,
+            '--provider-security-group', sg_1.id,
+            '--provider-security-group', sg_2.id,
+            'test-port',
+        ]
+        verifylist = [
+            ('network', self._port.network_id,),
+            ('enable', True),
+            ('provider_security_groups', [sg_1.id, sg_2.id]),
+            ('name', 'test-port'),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        columns, data = (self.cmd.take_action(parsed_args))
+
+        self.network.create_port.assert_called_once_with(**{
+            'admin_state_up': True,
+            'network_id': self._port.network_id,
+            'provider_security_groups': [sg_1.id, sg_2.id],
+            'name': 'test-port',
+        })
+
+        ref_columns, ref_data = self._get_common_cols_data(self._port)
+        self.assertEqual(ref_columns, columns)
+        self.assertEqual(ref_data, data)
+
+    def test_create_with_provider_security_group_by_name(self):
+        # create a port with 1 provider security group
+        secgroup = network_fakes.FakeSecurityGroup.create_one_security_group()
+        self.network.find_security_group = mock.Mock(return_value=secgroup)
+        arglist = [
+            '--network', self._port.network_id,
+            '--provider-security-group', secgroup.name,
+            'test-port',
+        ]
+        verifylist = [
+            ('network', self._port.network_id,),
+            ('enable', True),
+            ('provider_security_groups', [secgroup.name]),
+            ('name', 'test-port'),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        columns, data = (self.cmd.take_action(parsed_args))
+
+        self.network.create_port.assert_called_once_with(**{
+            'admin_state_up': True,
+            'network_id': self._port.network_id,
+            'provider_security_groups': [secgroup.id],
+            'name': 'test-port',
+        })
+
+        ref_columns, ref_data = self._get_common_cols_data(self._port)
+        self.assertEqual(ref_columns, columns)
+        self.assertEqual(ref_data, data)
+
+    def _test_create_with_flag_arg(
+        self, arg_name, validate_name, validate_val):
+        self.network.create_port.reset_mock()
+        # add '--' to the arg name and change '_' to '-'
+        conv_name = '--' + re.sub('_', '-', arg_name)
+        arglist = [
+            self._port.name,
+            '--network', self._port.network_id,
+            conv_name
+        ]
+        verifylist = [
+            ('name', self._port.name),
+            ('network', self._port.network_id,),
+            (arg_name, True),
+            ('enable', True),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        columns, data = (self.cmd.take_action(parsed_args))
+        self.network.create_port.assert_called_once_with(**{
+            'admin_state_up': True,
+            'network_id': self._port.network_id,
+            'name': self._port.name,
+            validate_name: validate_val,
+        })
+
+        ref_columns, ref_data = self._get_common_cols_data(self._port)
+        self.assertEqual(ref_columns, columns)
+        self.assertEqual(ref_data, data)
+
+    def test_create_with_mac_learning(self):
+        self._test_create_with_flag_arg(
+            'enable_mac_learning', 'mac_learning_enabled', True)
+
+    def test_create_with_no_mac_learning(self):
+        self._test_create_with_flag_arg(
+            'disable_mac_learning', 'mac_learning_enabled', False)
+
 
 class TestSetPort(test_port.TestSetPort):
 
@@ -125,4 +258,53 @@ class TestSetPort(test_port.TestSetPort):
         # check illegal index
         self._test_set_Vnic_index('illegal', is_valid=False)
 
-# TODO(asarfaty): add tests for provider-security-groups
+    def test_set_provider_security_group(self):
+        # It is not allowed to change the provider security groups
+        sg = network_fakes.FakeSecurityGroup.create_one_security_group()
+        self.network.find_security_group = mock.Mock(return_value=sg)
+        arglist = [
+            '--provider-security-group', sg.id,
+            self._port.name,
+        ]
+        verifylist = [
+            ('provider_security_groups', [sg.id]),
+            ('port', self._port.name),
+        ]
+
+        self.assertRaises(tests_utils.ParserException, self.check_parser,
+                          self.cmd, arglist, verifylist)
+
+    def _test_set_with_flag_arg(self, arg_name, validate_name,
+                                validate_val, is_valid=True):
+        self.network.update_port.reset_mock()
+        # add '--' to the arg name and change '_' to '-'
+        conv_name = '--' + re.sub('_', '-', arg_name)
+        arglist = [
+            self._port.name,
+            conv_name
+        ]
+        verifylist = [
+            ('port', self._port.name),
+            (arg_name, True)
+        ]
+        if not is_valid:
+            self.assertRaises(tests_utils.ParserException, self.check_parser,
+                              self.cmd, arglist, verifylist)
+            return
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        result = self.cmd.take_action(parsed_args)
+
+        attrs = {validate_name: validate_val}
+        self.network.update_port.assert_called_once_with(
+            self._port, **attrs)
+        self.assertIsNone(result)
+
+    def test_set_with_mac_learning(self):
+        self._test_set_with_flag_arg(
+            'enable_mac_learning', 'mac_learning_enabled', True)
+
+    def test_set_with_no_mac_learning(self):
+        self._test_set_with_flag_arg(
+            'disable_mac_learning', 'mac_learning_enabled', False)