Skip to content

Commit dd81ca0

Browse files
Jenkinsopenstack-gerrit
Jenkins
authored and
openstack-gerrit
committed
Merge "Simplify logic around option lists in port set"
2 parents c3fee25 + 82a86d2 commit dd81ca0

3 files changed

Lines changed: 182 additions & 126 deletions

File tree

‎openstackclient/network/v2/port.py‎

Lines changed: 47 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,6 @@ def _get_attrs(client_manager, parsed_args):
114114
))
115115
if parsed_args.description is not None:
116116
attrs['description'] = parsed_args.description
117-
if parsed_args.fixed_ip is not None:
118-
attrs['fixed_ips'] = parsed_args.fixed_ip
119117
if parsed_args.device:
120118
attrs['device_id'] = parsed_args.device
121119
if parsed_args.device_owner is not None:
@@ -124,8 +122,6 @@ def _get_attrs(client_manager, parsed_args):
124122
attrs['admin_state_up'] = True
125123
if parsed_args.disable:
126124
attrs['admin_state_up'] = False
127-
if parsed_args.binding_profile is not None:
128-
attrs['binding:profile'] = parsed_args.binding_profile
129125
if parsed_args.vnic_type is not None:
130126
attrs['binding:vnic_type'] = parsed_args.vnic_type
131127
if parsed_args.host:
@@ -389,13 +385,20 @@ def take_action(self, parsed_args):
389385
_prepare_fixed_ips(self.app.client_manager, parsed_args)
390386
attrs = _get_attrs(self.app.client_manager, parsed_args)
391387

388+
if parsed_args.binding_profile is not None:
389+
attrs['binding:profile'] = parsed_args.binding_profile
390+
391+
if parsed_args.fixed_ip:
392+
attrs['fixed_ips'] = parsed_args.fixed_ip
393+
392394
if parsed_args.security_group:
393395
attrs['security_group_ids'] = [client.find_security_group(
394396
sg, ignore_missing=False).id
395397
for sg in
396398
parsed_args.security_group]
397399
elif parsed_args.no_security_group:
398400
attrs['security_group_ids'] = []
401+
399402
if parsed_args.allowed_address_pairs:
400403
attrs['allowed_address_pairs'] = (
401404
_convert_address_pairs(parsed_args))
@@ -674,48 +677,50 @@ def take_action(self, parsed_args):
674677
_prepare_fixed_ips(self.app.client_manager, parsed_args)
675678
attrs = _get_attrs(self.app.client_manager, parsed_args)
676679
obj = client.find_port(parsed_args.port, ignore_missing=False)
677-
if 'binding:profile' in attrs:
678-
# Do not modify attrs if both binding_profile/no_binding given
679-
if not parsed_args.no_binding_profile:
680-
tmp_binding_profile = copy.deepcopy(obj.binding_profile)
681-
tmp_binding_profile.update(attrs['binding:profile'])
682-
attrs['binding:profile'] = tmp_binding_profile
683-
elif parsed_args.no_binding_profile:
684-
attrs['binding:profile'] = {}
685-
if 'fixed_ips' in attrs:
686-
# When user unsets the fixed_ips, obj.fixed_ips = [{}].
687-
# Adding the obj.fixed_ips list to attrs['fixed_ips']
688-
# would therefore add an empty dictionary, while we need
689-
# to append the attrs['fixed_ips'] iff there is some info
690-
# in the obj.fixed_ips. Therefore I have opted for this `for` loop
691-
# Do not modify attrs if fixed_ip/no_fixed_ip given
692-
if not parsed_args.no_fixed_ip:
693-
attrs['fixed_ips'] += [ip for ip in obj.fixed_ips if ip]
694-
elif parsed_args.no_fixed_ip:
695-
attrs['fixed_ips'] = []
696680

697-
if parsed_args.security_group:
698-
attrs['security_group_ids'] = [
699-
client.find_security_group(sg, ignore_missing=False).id for
700-
sg in parsed_args.security_group]
701-
if not parsed_args.no_security_group:
702-
attrs['security_group_ids'] += obj.security_group_ids
681+
if parsed_args.no_binding_profile:
682+
attrs['binding:profile'] = {}
683+
if parsed_args.binding_profile:
684+
if 'binding:profile' not in attrs:
685+
attrs['binding:profile'] = copy.deepcopy(obj.binding_profile)
686+
attrs['binding:profile'].update(parsed_args.binding_profile)
703687

704-
elif parsed_args.no_security_group:
688+
if parsed_args.no_fixed_ip:
689+
attrs['fixed_ips'] = []
690+
if parsed_args.fixed_ip:
691+
if 'fixed_ips' not in attrs:
692+
# obj.fixed_ips = [{}] if no fixed IPs are set.
693+
# Only append this to attrs['fixed_ips'] if actual fixed
694+
# IPs are present to avoid adding an empty dict.
695+
attrs['fixed_ips'] = [ip for ip in obj.fixed_ips if ip]
696+
attrs['fixed_ips'].extend(parsed_args.fixed_ip)
697+
698+
if parsed_args.no_security_group:
705699
attrs['security_group_ids'] = []
706-
707-
if (parsed_args.allowed_address_pairs and
708-
parsed_args.no_allowed_address_pair):
709-
attrs['allowed_address_pairs'] = (
710-
_convert_address_pairs(parsed_args))
711-
712-
elif parsed_args.allowed_address_pairs:
713-
attrs['allowed_address_pairs'] = (
714-
[addr for addr in obj.allowed_address_pairs if addr] +
715-
_convert_address_pairs(parsed_args))
716-
717-
elif parsed_args.no_allowed_address_pair:
700+
if parsed_args.security_group:
701+
if 'security_group_ids' not in attrs:
702+
# NOTE(dtroyer): Get existing security groups, iterate the
703+
# list to force a new list object to be
704+
# created and make sure the SDK Resource
705+
# marks the attribute 'dirty'.
706+
attrs['security_group_ids'] = [
707+
id for id in obj.security_group_ids
708+
]
709+
attrs['security_group_ids'].extend(
710+
client.find_security_group(sg, ignore_missing=False).id
711+
for sg in parsed_args.security_group
712+
)
713+
714+
if parsed_args.no_allowed_address_pair:
718715
attrs['allowed_address_pairs'] = []
716+
if parsed_args.allowed_address_pairs:
717+
if 'allowed_address_pairs' not in attrs:
718+
attrs['allowed_address_pairs'] = (
719+
[addr for addr in obj.allowed_address_pairs if addr]
720+
)
721+
attrs['allowed_address_pairs'].extend(
722+
_convert_address_pairs(parsed_args)
723+
)
719724

720725
client.update_port(obj, **attrs)
721726

‎openstackclient/tests/functional/network/v2/test_port.py‎

Lines changed: 75 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ class PortTests(base.TestCase):
2020
"""Functional tests for port. """
2121
NAME = uuid.uuid4().hex
2222
NETWORK_NAME = uuid.uuid4().hex
23-
SG_NAME = uuid.uuid4().hex
2423

2524
@classmethod
2625
def setUpClass(cls):
@@ -112,29 +111,32 @@ def test_port_list(self):
112111

113112
def test_port_set(self):
114113
"""Test create, set, show, delete"""
114+
name = uuid.uuid4().hex
115115
json_output = json.loads(self.openstack(
116116
'port create -f json ' +
117117
'--network ' + self.NETWORK_NAME + ' ' +
118-
'--description xyzpdq '
118+
'--description xyzpdq ' +
119119
'--disable ' +
120-
self.NAME
120+
name
121121
))
122122
id1 = json_output.get('id')
123123
self.addCleanup(self.openstack, 'port delete ' + id1)
124-
self.assertEqual(self.NAME, json_output.get('name'))
124+
self.assertEqual(name, json_output.get('name'))
125125
self.assertEqual('xyzpdq', json_output.get('description'))
126126
self.assertEqual('DOWN', json_output.get('admin_state_up'))
127127

128128
raw_output = self.openstack(
129-
'port set ' + '--enable ' + self.NAME)
129+
'port set ' + '--enable ' +
130+
name
131+
)
130132
self.assertOutput('', raw_output)
131133

132134
json_output = json.loads(self.openstack(
133-
'port show -f json ' + self.NAME
135+
'port show -f json ' + name
134136
))
135137
sg_id = json_output.get('security_group_ids')
136138

137-
self.assertEqual(self.NAME, json_output.get('name'))
139+
self.assertEqual(name, json_output.get('name'))
138140
self.assertEqual('xyzpdq', json_output.get('description'))
139141
self.assertEqual('UP', json_output.get('admin_state_up'))
140142
self.assertIsNotNone(json_output.get('mac_address'))
@@ -144,7 +146,7 @@ def test_port_set(self):
144146
self.assertOutput('', raw_output)
145147

146148
json_output = json.loads(self.openstack(
147-
'port show -f json ' + self.NAME
149+
'port show -f json ' + name
148150
))
149151
self.assertEqual('', json_output.get('security_group_ids'))
150152

@@ -166,3 +168,68 @@ def test_port_admin_set(self):
166168
'port show -f json ' + self.NAME
167169
))
168170
self.assertEqual(json_output.get('mac_address'), '11:22:33:44:55:66')
171+
172+
def test_port_set_sg(self):
173+
"""Test create, set, show, delete"""
174+
sg_name1 = uuid.uuid4().hex
175+
json_output = json.loads(self.openstack(
176+
'security group create -f json ' +
177+
sg_name1
178+
))
179+
sg_id1 = json_output.get('id')
180+
self.addCleanup(self.openstack, 'security group delete ' + sg_id1)
181+
182+
sg_name2 = uuid.uuid4().hex
183+
json_output = json.loads(self.openstack(
184+
'security group create -f json ' +
185+
sg_name2
186+
))
187+
sg_id2 = json_output.get('id')
188+
self.addCleanup(self.openstack, 'security group delete ' + sg_id2)
189+
190+
name = uuid.uuid4().hex
191+
json_output = json.loads(self.openstack(
192+
'port create -f json ' +
193+
'--network ' + self.NETWORK_NAME + ' ' +
194+
'--security-group ' + sg_name1 + ' ' +
195+
name
196+
))
197+
id1 = json_output.get('id')
198+
self.addCleanup(self.openstack, 'port delete ' + id1)
199+
self.assertEqual(name, json_output.get('name'))
200+
self.assertEqual(sg_id1, json_output.get('security_group_ids'))
201+
202+
raw_output = self.openstack(
203+
'port set ' +
204+
'--security-group ' + sg_name2 + ' ' +
205+
name
206+
)
207+
self.assertOutput('', raw_output)
208+
209+
json_output = json.loads(self.openstack(
210+
'port show -f json ' + name
211+
))
212+
self.assertEqual(name, json_output.get('name'))
213+
self.assertIn(
214+
# TODO(dtroyer): output formatters should not mess with JSON!
215+
sg_id1,
216+
json_output.get('security_group_ids'),
217+
)
218+
self.assertIn(
219+
# TODO(dtroyer): output formatters should not mess with JSON!
220+
sg_id2,
221+
json_output.get('security_group_ids'),
222+
)
223+
224+
raw_output = self.openstack(
225+
'port unset --security-group ' + sg_id1 + ' ' + id1)
226+
self.assertOutput('', raw_output)
227+
228+
json_output = json.loads(self.openstack(
229+
'port show -f json ' + name
230+
))
231+
self.assertEqual(
232+
# TODO(dtroyer): output formatters should do this on JSON!
233+
sg_id2,
234+
json_output.get('security_group_ids'),
235+
)

0 commit comments

Comments
 (0)