Simplify logic around option lists in port set

Use a common pattern to handle option pairs --XYZ and --no-XYZ for managing
lists of attributes. This pattern looks at the presence of the option
in parsed_args first and branches as necessary.

Some specific steps are included for the SDK Network resources to reliably
set the 'dirty' flag for changed attributes via one or both of the following:
* iterate over lists of original resource attributes to force the creation
  of a new list object
* use [].extend() rather than += to add to the existing list (substitute
  {}.update() for dicts)

Change-Id: I0c3f9a52ffe1ae2b5b230cb13d6376dd9131aaf9

1 files changed, 47 insertions, 42 deletions

diff --git a/openstackclient/network/v2/port.py b/openstackclient/network/v2/port.py
index eced93ce..f77f566d 100644
--- a/openstackclient/network/v2/port.py
+++ b/openstackclient/network/v2/port.py
@@ -114,8 +114,6 @@ def _get_attrs(client_manager, parsed_args):
         ))
     if parsed_args.description is not None:
         attrs['description'] = parsed_args.description
-    if parsed_args.fixed_ip is not None:
-        attrs['fixed_ips'] = parsed_args.fixed_ip
     if parsed_args.device:
         attrs['device_id'] = parsed_args.device
     if parsed_args.device_owner is not None:
@@ -124,8 +122,6 @@ def _get_attrs(client_manager, parsed_args):
         attrs['admin_state_up'] = True
     if parsed_args.disable:
         attrs['admin_state_up'] = False
-    if parsed_args.binding_profile is not None:
-        attrs['binding:profile'] = parsed_args.binding_profile
     if parsed_args.vnic_type is not None:
         attrs['binding:vnic_type'] = parsed_args.vnic_type
     if parsed_args.host:
@@ -389,6 +385,12 @@ class CreatePort(command.ShowOne):
         _prepare_fixed_ips(self.app.client_manager, parsed_args)
         attrs = _get_attrs(self.app.client_manager, parsed_args)
 
+        if parsed_args.binding_profile is not None:
+            attrs['binding:profile'] = parsed_args.binding_profile
+
+        if parsed_args.fixed_ip:
+            attrs['fixed_ips'] = parsed_args.fixed_ip
+
         if parsed_args.security_group:
             attrs['security_group_ids'] = [client.find_security_group(
                                            sg, ignore_missing=False).id
@@ -396,6 +398,7 @@ class CreatePort(command.ShowOne):
                                            parsed_args.security_group]
         elif parsed_args.no_security_group:
             attrs['security_group_ids'] = []
+
         if parsed_args.allowed_address_pairs:
             attrs['allowed_address_pairs'] = (
                 _convert_address_pairs(parsed_args))
@@ -674,48 +677,50 @@ class SetPort(command.Command):
         _prepare_fixed_ips(self.app.client_manager, parsed_args)
         attrs = _get_attrs(self.app.client_manager, parsed_args)
         obj = client.find_port(parsed_args.port, ignore_missing=False)
-        if 'binding:profile' in attrs:
-            # 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:
-            # When user unsets the fixed_ips, obj.fixed_ips = [{}].
-            # Adding the obj.fixed_ips list to attrs['fixed_ips']
-            # 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
-            # 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'] = []
 
-        if parsed_args.security_group:
-            attrs['security_group_ids'] = [
-                client.find_security_group(sg, ignore_missing=False).id for
-                sg in parsed_args.security_group]
-            if not parsed_args.no_security_group:
-                attrs['security_group_ids'] += obj.security_group_ids
+        if parsed_args.no_binding_profile:
+            attrs['binding:profile'] = {}
+        if parsed_args.binding_profile:
+            if 'binding:profile' not in attrs:
+                attrs['binding:profile'] = copy.deepcopy(obj.binding_profile)
+            attrs['binding:profile'].update(parsed_args.binding_profile)
 
-        elif parsed_args.no_security_group:
+        if parsed_args.no_fixed_ip:
+            attrs['fixed_ips'] = []
+        if parsed_args.fixed_ip:
+            if 'fixed_ips' not in attrs:
+                # obj.fixed_ips = [{}] if no fixed IPs are set.
+                # Only append this to attrs['fixed_ips'] if actual fixed
+                # IPs are present to avoid adding an empty dict.
+                attrs['fixed_ips'] = [ip for ip in obj.fixed_ips if ip]
+            attrs['fixed_ips'].extend(parsed_args.fixed_ip)
+
+        if parsed_args.no_security_group:
             attrs['security_group_ids'] = []
-
-        if (parsed_args.allowed_address_pairs and
-                parsed_args.no_allowed_address_pair):
-            attrs['allowed_address_pairs'] = (
-                _convert_address_pairs(parsed_args))
-
-        elif parsed_args.allowed_address_pairs:
-            attrs['allowed_address_pairs'] = (
-                [addr for addr in obj.allowed_address_pairs if addr] +
-                _convert_address_pairs(parsed_args))
-
-        elif parsed_args.no_allowed_address_pair:
+        if parsed_args.security_group:
+            if 'security_group_ids' not in attrs:
+                # NOTE(dtroyer): Get existing security groups, iterate the
+                #                list to force a new list object to be
+                #                created and make sure the SDK Resource
+                #                marks the attribute 'dirty'.
+                attrs['security_group_ids'] = [
+                    id for id in obj.security_group_ids
+                ]
+            attrs['security_group_ids'].extend(
+                client.find_security_group(sg, ignore_missing=False).id
+                for sg in parsed_args.security_group
+            )
+
+        if parsed_args.no_allowed_address_pair:
             attrs['allowed_address_pairs'] = []
+        if parsed_args.allowed_address_pairs:
+            if 'allowed_address_pairs' not in attrs:
+                attrs['allowed_address_pairs'] = (
+                    [addr for addr in obj.allowed_address_pairs if addr]
+                )
+            attrs['allowed_address_pairs'].extend(
+                _convert_address_pairs(parsed_args)
+            )
 
         client.update_port(obj, **attrs)