Merge "Introduce overwrite functionality in ``osc port set``"

4 files changed, 78 insertions, 15 deletions

diff --git a/doc/source/command-objects/port.rst b/doc/source/command-objects/port.rst
index 04cf59a2..e3e783ad 100644
--- a/doc/source/command-objects/port.rst
+++ b/doc/source/command-objects/port.rst
@@ -144,11 +144,13 @@ Set port properties
 .. code:: bash
 
     os port set
-        [--fixed-ip subnet=<subnet>,ip-address=<ip-address> | --no-fixed-ip]
+        [--fixed-ip subnet=<subnet>,ip-address=<ip-address>]
+        [--no-fixed-ip]
         [--device <device-id>]
         [--device-owner <device-owner>]
         [--vnic-type <vnic-type>]
-        [--binding-profile <binding-profile> | --no-binding-profile]
+        [--binding-profile <binding-profile>]
+        [--no-binding-profile]
         [--host <host-id>]
         [--enable | --disable]
         [--name <name>]
@@ -162,7 +164,9 @@ Set port properties
 
 .. option:: --no-fixed-ip
 
-    Clear existing information of fixed IP addresses
+    Clear existing information of fixed IP addresses.
+    Specify both --fixed-ip and --no-fixed-ip
+    to overwrite the current fixed IP addresses.
 
 .. option:: --device <device-id>
 
@@ -186,7 +190,9 @@ Set port properties
 
 .. option:: --no-binding-profile
 
-    Clear existing information of binding:profile
+    Clear existing information of binding:profile.
+    Specify both --binding-profile and --no-binding-profile
+    to overwrite the current binding:profile information.
 
 .. option:: --host <host-id>
 
diff --git a/openstackclient/network/v2/port.py b/openstackclient/network/v2/port.py
index 0df78e43..92b286a9 100644
--- a/openstackclient/network/v2/port.py
+++ b/openstackclient/network/v2/port.py
@@ -109,7 +109,6 @@ def _get_attrs(client_manager, parsed_args):
             'The --host-id option is deprecated, '
             'please use --host instead.'
         ))
-
     if parsed_args.fixed_ip is not None:
         attrs['fixed_ips'] = parsed_args.fixed_ip
     if parsed_args.device:
@@ -428,8 +427,7 @@ class SetPort(command.Command):
             metavar="<name>",
             help=_("Set port name")
         )
-        fixed_ip = parser.add_mutually_exclusive_group()
-        fixed_ip.add_argument(
+        parser.add_argument(
             '--fixed-ip',
             metavar='subnet=<subnet>,ip-address=<ip-address>',
             action=parseractions.MultiKeyValueAction,
@@ -438,13 +436,14 @@ class SetPort(command.Command):
                    "subnet=<subnet>,ip-address=<ip-address> "
                    "(repeat option to set multiple fixed IP addresses)")
         )
-        fixed_ip.add_argument(
+        parser.add_argument(
             '--no-fixed-ip',
             action='store_true',
-            help=_("Clear existing information of fixed IP addresses")
+            help=_("Clear existing information of fixed IP addresses."
+                   "Specify both --fixed-ip and --no-fixed-ip "
+                   "to overwrite the current fixed IP addresses.")
         )
-        binding_profile = parser.add_mutually_exclusive_group()
-        binding_profile.add_argument(
+        parser.add_argument(
             '--binding-profile',
             metavar='<binding-profile>',
             action=JSONKeyValueAction,
@@ -452,10 +451,12 @@ class SetPort(command.Command):
                    "be passed as <key>=<value> or JSON. "
                    "(repeat option to set multiple binding:profile data)")
         )
-        binding_profile.add_argument(
+        parser.add_argument(
             '--no-binding-profile',
             action='store_true',
-            help=_("Clear existing information of binding:profile")
+            help=_("Clear existing information of binding:profile."
+                   "Specify both --binding-profile and --no-binding-profile "
+                   "to overwrite the current binding:profile information.")
         )
         parser.add_argument(
             'port',
@@ -471,7 +472,11 @@ class SetPort(command.Command):
         attrs = _get_attrs(self.app.client_manager, parsed_args)
         obj = client.find_port(parsed_args.port, ignore_missing=False)
         if 'binding:profile' in attrs:
-            attrs['binding:profile'].update(obj.binding_profile)
+            # Do not modify attrs if both binding_profile/no_binding given
+            if not parsed_args.no_binding_profile:
+                tmp_binding_profile = copy.deepcopy(obj.binding_profile)
+                tmp_binding_profile.update(attrs['binding:profile'])
+                attrs['binding:profile'] = tmp_binding_profile
         elif parsed_args.no_binding_profile:
             attrs['binding:profile'] = {}
         if 'fixed_ips' in attrs:
@@ -480,7 +485,9 @@ class SetPort(command.Command):
             # would therefore add an empty dictionary, while we need
             # to append the attrs['fixed_ips'] iff there is some info
             # in the obj.fixed_ips. Therefore I have opted for this `for` loop
-            attrs['fixed_ips'] += [ip for ip in obj.fixed_ips if ip]
+            # Do not modify attrs if fixed_ip/no_fixed_ip given
+            if not parsed_args.no_fixed_ip:
+                attrs['fixed_ips'] += [ip for ip in obj.fixed_ips if ip]
         elif parsed_args.no_fixed_ip:
             attrs['fixed_ips'] = []
 
diff --git a/openstackclient/tests/unit/network/v2/test_port.py b/openstackclient/tests/unit/network/v2/test_port.py
index 271e8160..a2aceab1 100644
--- a/openstackclient/tests/unit/network/v2/test_port.py
+++ b/openstackclient/tests/unit/network/v2/test_port.py
@@ -494,6 +494,50 @@ class TestSetPort(TestPort):
         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)
+        arglist = [
+            '--binding-profile', 'lok_i=than_os',
+            '--no-binding-profile',
+            _testport.name,
+        ]
+        verifylist = [
+            ('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 = {
+            'binding:profile':
+                {'lok_i': 'than_os'},
+        }
+        self.network.update_port.assert_called_once_with(_testport, **attrs)
+        self.assertIsNone(result)
+
+    def test_overwrite_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 = [
+            '--fixed-ip', 'ip-address=10.0.0.12',
+            '--no-fixed-ip',
+            _testport.name,
+        ]
+        verifylist = [
+            ('fixed_ip', [{'ip-address': '10.0.0.12'}]),
+            ('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'}],
+        }
+        self.network.update_port.assert_called_once_with(_testport, **attrs)
+        self.assertIsNone(result)
+
     def test_set_this(self):
         arglist = [
             '--disable',
diff --git a/releasenotes/notes/add-overwrite-option-190a9c6904d53dab.yaml b/releasenotes/notes/add-overwrite-option-190a9c6904d53dab.yaml
new file mode 100644
index 00000000..fdd61b52
--- /dev/null
+++ b/releasenotes/notes/add-overwrite-option-190a9c6904d53dab.yaml
@@ -0,0 +1,6 @@
+---
+features:
+  - |
+    ``port set`` command now allows the user to overwrite fixed-ips or binding-profile
+    of a port.
+    [ Blueprint  `allow-overwrite-set-options <https://blueprints.launchpad.net/python-openstackclient/+spec/allow-overwrite-set-options>` _]
\ No newline at end of file