Merge "Simplify logic around option lists in port set"

2 files changed, 135 insertions, 84 deletions

diff --git a/openstackclient/tests/functional/network/v2/test_port.py b/openstackclient/tests/functional/network/v2/test_port.py
index bd5eefa5..62c0cbe5 100644
--- a/openstackclient/tests/functional/network/v2/test_port.py
+++ b/openstackclient/tests/functional/network/v2/test_port.py
@@ -20,7 +20,6 @@ class PortTests(base.TestCase):
     """Functional tests for port. """
     NAME = uuid.uuid4().hex
     NETWORK_NAME = uuid.uuid4().hex
-    SG_NAME = uuid.uuid4().hex
 
     @classmethod
     def setUpClass(cls):
@@ -112,29 +111,32 @@ class PortTests(base.TestCase):
 
     def test_port_set(self):
         """Test create, set, show, delete"""
+        name = uuid.uuid4().hex
         json_output = json.loads(self.openstack(
             'port create -f json ' +
             '--network ' + self.NETWORK_NAME + ' ' +
-            '--description xyzpdq '
+            '--description xyzpdq ' +
             '--disable ' +
-            self.NAME
+            name
         ))
         id1 = json_output.get('id')
         self.addCleanup(self.openstack, 'port delete ' + id1)
-        self.assertEqual(self.NAME, json_output.get('name'))
+        self.assertEqual(name, json_output.get('name'))
         self.assertEqual('xyzpdq', json_output.get('description'))
         self.assertEqual('DOWN', json_output.get('admin_state_up'))
 
         raw_output = self.openstack(
-            'port set ' + '--enable ' + self.NAME)
+            'port set ' + '--enable ' +
+            name
+        )
         self.assertOutput('', raw_output)
 
         json_output = json.loads(self.openstack(
-            'port show -f json ' + self.NAME
+            'port show -f json ' + name
         ))
         sg_id = json_output.get('security_group_ids')
 
-        self.assertEqual(self.NAME, json_output.get('name'))
+        self.assertEqual(name, json_output.get('name'))
         self.assertEqual('xyzpdq', json_output.get('description'))
         self.assertEqual('UP', json_output.get('admin_state_up'))
         self.assertIsNotNone(json_output.get('mac_address'))
@@ -144,7 +146,7 @@ class PortTests(base.TestCase):
         self.assertOutput('', raw_output)
 
         json_output = json.loads(self.openstack(
-            'port show -f json ' + self.NAME
+            'port show -f json ' + name
         ))
         self.assertEqual('', json_output.get('security_group_ids'))
 
@@ -166,3 +168,68 @@ class PortTests(base.TestCase):
             'port show -f json ' + self.NAME
         ))
         self.assertEqual(json_output.get('mac_address'), '11:22:33:44:55:66')
+
+    def test_port_set_sg(self):
+        """Test create, set, show, delete"""
+        sg_name1 = uuid.uuid4().hex
+        json_output = json.loads(self.openstack(
+            'security group create -f json ' +
+            sg_name1
+        ))
+        sg_id1 = json_output.get('id')
+        self.addCleanup(self.openstack, 'security group delete ' + sg_id1)
+
+        sg_name2 = uuid.uuid4().hex
+        json_output = json.loads(self.openstack(
+            'security group create -f json ' +
+            sg_name2
+        ))
+        sg_id2 = json_output.get('id')
+        self.addCleanup(self.openstack, 'security group delete ' + sg_id2)
+
+        name = uuid.uuid4().hex
+        json_output = json.loads(self.openstack(
+            'port create -f json ' +
+            '--network ' + self.NETWORK_NAME + ' ' +
+            '--security-group ' + sg_name1 + ' ' +
+            name
+        ))
+        id1 = json_output.get('id')
+        self.addCleanup(self.openstack, 'port delete ' + id1)
+        self.assertEqual(name, json_output.get('name'))
+        self.assertEqual(sg_id1, json_output.get('security_group_ids'))
+
+        raw_output = self.openstack(
+            'port set ' +
+            '--security-group ' + sg_name2 + ' ' +
+            name
+        )
+        self.assertOutput('', raw_output)
+
+        json_output = json.loads(self.openstack(
+            'port show -f json ' + name
+        ))
+        self.assertEqual(name, json_output.get('name'))
+        self.assertIn(
+            # TODO(dtroyer): output formatters should not mess with JSON!
+            sg_id1,
+            json_output.get('security_group_ids'),
+        )
+        self.assertIn(
+            # TODO(dtroyer): output formatters should not mess with JSON!
+            sg_id2,
+            json_output.get('security_group_ids'),
+        )
+
+        raw_output = self.openstack(
+            'port unset --security-group ' + sg_id1 + ' ' + id1)
+        self.assertOutput('', raw_output)
+
+        json_output = json.loads(self.openstack(
+            'port show -f json ' + name
+        ))
+        self.assertEqual(
+            # TODO(dtroyer): output formatters should do this on JSON!
+            sg_id2,
+            json_output.get('security_group_ids'),
+        )
diff --git a/openstackclient/tests/unit/network/v2/test_port.py b/openstackclient/tests/unit/network/v2/test_port.py
index d2df5841..701af879 100644
--- a/openstackclient/tests/unit/network/v2/test_port.py
+++ b/openstackclient/tests/unit/network/v2/test_port.py
@@ -883,106 +883,105 @@ class TestSetPort(TestPort):
         # Get the command object to test
         self.cmd = port.SetPort(self.app, self.namespace)
 
-    def test_set_fixed_ip(self):
+    def test_set_port_defaults(self):
         arglist = [
-            '--fixed-ip', 'ip-address=10.0.0.11',
             self._port.name,
-            '--no-fixed-ip',
         ]
         verifylist = [
-            ('fixed_ip', [{'ip-address': '10.0.0.11'}]),
             ('port', self._port.name),
         ]
-
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
-        result = self.cmd.take_action(parsed_args)
 
-        attrs = {
-            'fixed_ips': [{'ip_address': '10.0.0.11'}],
-        }
+        result = self.cmd.take_action(parsed_args)
+        attrs = {}
         self.network.update_port.assert_called_once_with(self._port, **attrs)
         self.assertIsNone(result)
 
-    def test_set_dns_name(self):
+    def test_set_port_fixed_ip(self):
+        _testport = network_fakes.FakePort.create_one_port(
+            {'fixed_ips': [{'ip_address': '0.0.0.1'}]})
+        self.network.find_port = mock.Mock(return_value=_testport)
         arglist = [
-            '--dns-name', '8.8.8.8',
-            self._port.name,
+            '--fixed-ip', 'ip-address=10.0.0.12',
+            _testport.name,
         ]
         verifylist = [
-            ('dns_name', '8.8.8.8'),
-            ('port', self._port.name),
+            ('fixed_ip', [{'ip-address': '10.0.0.12'}]),
+            ('port', _testport.name),
         ]
-
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
-        result = self.cmd.take_action(parsed_args)
 
+        result = self.cmd.take_action(parsed_args)
         attrs = {
-            'dns_name': '8.8.8.8',
+            'fixed_ips': [
+                {'ip_address': '0.0.0.1'},
+                {'ip_address': '10.0.0.12'},
+            ],
         }
-        self.network.update_port.assert_called_once_with(self._port, **attrs)
+        self.network.update_port.assert_called_once_with(_testport, **attrs)
         self.assertIsNone(result)
 
-    def test_append_fixed_ip(self):
+    def test_set_port_fixed_ip_clear(self):
         _testport = network_fakes.FakePort.create_one_port(
             {'fixed_ips': [{'ip_address': '0.0.0.1'}]})
         self.network.find_port = mock.Mock(return_value=_testport)
         arglist = [
             '--fixed-ip', 'ip-address=10.0.0.12',
+            '--no-fixed-ip',
             _testport.name,
         ]
         verifylist = [
             ('fixed_ip', [{'ip-address': '10.0.0.12'}]),
-            ('port', _testport.name),
+            ('no_fixed_ip', True)
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
         result = self.cmd.take_action(parsed_args)
         attrs = {
             'fixed_ips': [
-                {'ip_address': '10.0.0.12'}, {'ip_address': '0.0.0.1'}],
+                {'ip_address': '10.0.0.12'},
+            ],
         }
         self.network.update_port.assert_called_once_with(_testport, **attrs)
         self.assertIsNone(result)
 
-    def test_overwrite_binding_profile(self):
-        _testport = network_fakes.FakePort.create_one_port(
-            {'binding_profile': {'lok_i': 'visi_on'}})
-        self.network.find_port = mock.Mock(return_value=_testport)
+    def test_set_port_dns_name(self):
         arglist = [
-            '--binding-profile', 'lok_i=than_os',
-            '--no-binding-profile',
-            _testport.name,
+            '--dns-name', '8.8.8.8',
+            self._port.name,
         ]
         verifylist = [
-            ('binding_profile', {'lok_i': 'than_os'}),
-            ('no_binding_profile', True)
+            ('dns_name', '8.8.8.8'),
+            ('port', self._port.name),
         ]
+
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
         result = self.cmd.take_action(parsed_args)
+
         attrs = {
-            'binding:profile':
-                {'lok_i': 'than_os'},
+            'dns_name': '8.8.8.8',
         }
-        self.network.update_port.assert_called_once_with(_testport, **attrs)
+        self.network.update_port.assert_called_once_with(self._port, **attrs)
         self.assertIsNone(result)
 
-    def test_overwrite_fixed_ip(self):
+    def test_set_port_overwrite_binding_profile(self):
         _testport = network_fakes.FakePort.create_one_port(
-            {'fixed_ips': [{'ip_address': '0.0.0.1'}]})
+            {'binding_profile': {'lok_i': 'visi_on'}})
         self.network.find_port = mock.Mock(return_value=_testport)
         arglist = [
-            '--fixed-ip', 'ip-address=10.0.0.12',
-            '--no-fixed-ip',
+            '--binding-profile', 'lok_i=than_os',
+            '--no-binding-profile',
             _testport.name,
         ]
         verifylist = [
-            ('fixed_ip', [{'ip-address': '10.0.0.12'}]),
-            ('no_fixed_ip', True)
+            ('binding_profile', {'lok_i': 'than_os'}),
+            ('no_binding_profile', True)
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
         result = self.cmd.take_action(parsed_args)
         attrs = {
-            'fixed_ips': [
-                {'ip_address': '10.0.0.12'}],
+            'binding:profile':
+                {'lok_i': 'than_os'},
         }
         self.network.update_port.assert_called_once_with(_testport, **attrs)
         self.assertIsNone(result)
@@ -1006,7 +1005,7 @@ class TestSetPort(TestPort):
         self.network.update_port.assert_called_once_with(_testport, **attrs)
         self.assertIsNone(result)
 
-    def test_set_this(self):
+    def test_set_port_this(self):
         arglist = [
             '--disable',
             '--no-fixed-ip',
@@ -1031,7 +1030,7 @@ class TestSetPort(TestPort):
         self.network.update_port.assert_called_once_with(self._port, **attrs)
         self.assertIsNone(result)
 
-    def test_set_that(self):
+    def test_set_port_that(self):
         arglist = [
             '--description', 'newDescription',
             '--enable',
@@ -1065,22 +1064,7 @@ class TestSetPort(TestPort):
         self.network.update_port.assert_called_once_with(self._port, **attrs)
         self.assertIsNone(result)
 
-    def test_set_nothing(self):
-        arglist = [
-            self._port.name,
-        ]
-        verifylist = [
-            ('port', self._port.name),
-        ]
-
-        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
-        result = self.cmd.take_action(parsed_args)
-
-        attrs = {}
-        self.network.update_port.assert_called_once_with(self._port, **attrs)
-        self.assertIsNone(result)
-
-    def test_set_invalid_json_binding_profile(self):
+    def test_set_port_invalid_json_binding_profile(self):
         arglist = [
             '--binding-profile', '{"parent_name"}',
             'test-port',
@@ -1091,7 +1075,7 @@ class TestSetPort(TestPort):
                           arglist,
                           None)
 
-    def test_set_invalid_key_value_binding_profile(self):
+    def test_set_port_invalid_key_value_binding_profile(self):
         arglist = [
             '--binding-profile', 'key',
             'test-port',
@@ -1102,7 +1086,7 @@ class TestSetPort(TestPort):
                           arglist,
                           None)
 
-    def test_set_mixed_binding_profile(self):
+    def test_set_port_mixed_binding_profile(self):
         arglist = [
             '--binding-profile', 'foo=bar',
             '--binding-profile', '{"foo2": "bar2"}',
@@ -1122,7 +1106,7 @@ class TestSetPort(TestPort):
         self.network.update_port.assert_called_once_with(self._port, **attrs)
         self.assertIsNone(result)
 
-    def test_set_security_group(self):
+    def test_set_port_security_group(self):
         sg = network_fakes.FakeSecurityGroup.create_one_security_group()
         self.network.find_security_group = mock.Mock(return_value=sg)
         arglist = [
@@ -1133,17 +1117,16 @@ class TestSetPort(TestPort):
             ('security_group', [sg.id]),
             ('port', self._port.name),
         ]
-
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
-        result = self.cmd.take_action(parsed_args)
 
+        result = self.cmd.take_action(parsed_args)
         attrs = {
             'security_group_ids': [sg.id],
         }
         self.network.update_port.assert_called_once_with(self._port, **attrs)
         self.assertIsNone(result)
 
-    def test_append_security_group(self):
+    def test_set_port_security_group_append(self):
         sg_1 = network_fakes.FakeSecurityGroup.create_one_security_group()
         sg_2 = network_fakes.FakeSecurityGroup.create_one_security_group()
         sg_3 = network_fakes.FakeSecurityGroup.create_one_security_group()
@@ -1161,14 +1144,15 @@ class TestSetPort(TestPort):
             ('port', _testport.name),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
         result = self.cmd.take_action(parsed_args)
         attrs = {
-            'security_group_ids': [sg_2.id, sg_3.id, sg_1.id],
+            'security_group_ids': [sg_1.id, sg_2.id, sg_3.id],
         }
         self.network.update_port.assert_called_once_with(_testport, **attrs)
         self.assertIsNone(result)
 
-    def test_set_no_security_groups(self):
+    def test_set_port_security_group_clear(self):
         arglist = [
             '--no-security-group',
             self._port.name,
@@ -1177,17 +1161,16 @@ class TestSetPort(TestPort):
             ('no_security_group', True),
             ('port', self._port.name),
         ]
-
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
-        result = self.cmd.take_action(parsed_args)
 
+        result = self.cmd.take_action(parsed_args)
         attrs = {
             'security_group_ids': [],
         }
         self.network.update_port.assert_called_once_with(self._port, **attrs)
         self.assertIsNone(result)
 
-    def test_overwrite_security_group(self):
+    def test_set_port_security_group_replace(self):
         sg1 = network_fakes.FakeSecurityGroup.create_one_security_group()
         sg2 = network_fakes.FakeSecurityGroup.create_one_security_group()
         _testport = network_fakes.FakePort.create_one_port(
@@ -1204,6 +1187,7 @@ class TestSetPort(TestPort):
             ('no_security_group', True)
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
         result = self.cmd.take_action(parsed_args)
         attrs = {
             'security_group_ids': [sg2.id],
@@ -1211,7 +1195,7 @@ class TestSetPort(TestPort):
         self.network.update_port.assert_called_once_with(_testport, **attrs)
         self.assertIsNone(result)
 
-    def test_set_allowed_address_pair(self):
+    def test_set_port_allowed_address_pair(self):
         arglist = [
             '--allowed-address', 'ip-address=192.168.1.123',
             self._port.name,
@@ -1230,7 +1214,7 @@ class TestSetPort(TestPort):
         self.network.update_port.assert_called_once_with(self._port, **attrs)
         self.assertIsNone(result)
 
-    def test_append_allowed_address_pair(self):
+    def test_set_port_append_allowed_address_pair(self):
         _testport = network_fakes.FakePort.create_one_port(
             {'allowed_address_pairs': [{'ip_address': '192.168.1.123'}]})
         self.network.find_port = mock.Mock(return_value=_testport)
@@ -1253,7 +1237,7 @@ class TestSetPort(TestPort):
         self.network.update_port.assert_called_once_with(_testport, **attrs)
         self.assertIsNone(result)
 
-    def test_overwrite_allowed_address_pair(self):
+    def test_set_port_overwrite_allowed_address_pair(self):
         _testport = network_fakes.FakePort.create_one_port(
             {'allowed_address_pairs': [{'ip_address': '192.168.1.123'}]})
         self.network.find_port = mock.Mock(return_value=_testport)
@@ -1277,7 +1261,7 @@ class TestSetPort(TestPort):
         self.network.update_port.assert_called_once_with(_testport, **attrs)
         self.assertIsNone(result)
 
-    def test_set_no_allowed_address_pairs(self):
+    def test_set_port_no_allowed_address_pairs(self):
         arglist = [
             '--no-allowed-address',
             self._port.name,
@@ -1296,7 +1280,7 @@ class TestSetPort(TestPort):
         self.network.update_port.assert_called_once_with(self._port, **attrs)
         self.assertIsNone(result)
 
-    def test_port_security_enabled(self):
+    def test_set_port_security_enabled(self):
         arglist = [
             '--enable-port-security',
             self._port.id,
@@ -1314,7 +1298,7 @@ class TestSetPort(TestPort):
             'port_security_enabled': True,
         })
 
-    def test_port_security_disabled(self):
+    def test_set_port_security_disabled(self):
         arglist = [
             '--disable-port-security',
             self._port.id,