Perform port update if security group changed
If only the security groups for a network changed in a profile, do not delete and recreate the network during a profile update. Instead apply the security group change by an update to the existing port. Closes-Bug: #1905490 Change-Id: I43f1181c1592144d49784ba0d3c51c01bf2f1060
This commit is contained in:
Duc Truong
parent
028af9be62
commit
6dff71e13f
|
|
@ -1332,6 +1332,43 @@ class ServerProfile(base.Profile):
|
|||
obj.data['internal_ports'] = internal_ports
|
||||
node_obj.Node.update(self.context, obj.id, {'data': obj.data})
|
||||
|
||||
def _nw_compare(self, n1, n2, property):
|
||||
return n1.get(property, None) == n2.get(property, None)
|
||||
|
||||
def _update_network_update_port(self, obj, networks):
|
||||
"""Update existing port in network from the node.
|
||||
|
||||
Currently only update to security group is supported.
|
||||
|
||||
:param obj: The node object to operate.
|
||||
:param networks: A list networks that contain updated security groups.
|
||||
:returns: ``None``
|
||||
:raises: ``EResourceUpdate``
|
||||
"""
|
||||
nc = self.network(obj)
|
||||
internal_ports = obj.data.get('internal_ports', [])
|
||||
|
||||
# process each network to be updated
|
||||
for n in networks:
|
||||
# verify network properties and resolve names into ids
|
||||
net = self._validate_network(obj, n, 'update')
|
||||
|
||||
# find existing port that matches network
|
||||
candidate_ports = self._find_port_by_net_spec(
|
||||
obj, net, internal_ports)
|
||||
port = candidate_ports[0]
|
||||
try:
|
||||
# set updated security groups for port
|
||||
port_attr = {
|
||||
'security_groups': net.get(self.PORT_SECURITY_GROUPS, []),
|
||||
}
|
||||
LOG.debug("Setting security groups %s for port %s",
|
||||
port_attr, port['id'])
|
||||
nc.port_update(port['id'], **port_attr)
|
||||
except exc.InternalError as ex:
|
||||
raise exc.EResourceUpdate(type='server', id=obj.physical_id,
|
||||
message=str(ex))
|
||||
|
||||
def _update_network(self, obj, new_profile):
|
||||
"""Updating server network interfaces.
|
||||
|
||||
|
|
@ -1344,10 +1381,36 @@ class ServerProfile(base.Profile):
|
|||
networks_current = self.properties[self.NETWORKS]
|
||||
networks_create = new_profile.properties[self.NETWORKS]
|
||||
networks_delete = copy.deepcopy(networks_current)
|
||||
for network in networks_current:
|
||||
if network in networks_create:
|
||||
networks_create.remove(network)
|
||||
networks_delete.remove(network)
|
||||
networks_update = []
|
||||
|
||||
for nw in networks_current:
|
||||
if nw in networks_create:
|
||||
# network already exist. no need to create or delete it.
|
||||
LOG.debug("Network %s already exists, skip create/delete", nw)
|
||||
networks_create.remove(nw)
|
||||
networks_delete.remove(nw)
|
||||
|
||||
# find networks for which only security group changed
|
||||
for nw in networks_current:
|
||||
# networks to be created with only sg changes
|
||||
sg_create_nw = [n for n in networks_create
|
||||
if (self._nw_compare(n, nw, 'network') and
|
||||
self._nw_compare(n, nw, 'port') and
|
||||
self._nw_compare(n, nw, 'fixed_ip') and
|
||||
self._nw_compare(n, nw, 'floating_network') and
|
||||
self._nw_compare(n, nw, 'floating_ip'))]
|
||||
for n in sg_create_nw:
|
||||
# don't create networks with only security group changes
|
||||
LOG.debug("Network %s only has security group changes, "
|
||||
"don't create/delete it. Only update it.", n)
|
||||
networks_create.remove(n)
|
||||
networks_update.append(n)
|
||||
if nw in networks_delete:
|
||||
networks_delete.remove(nw)
|
||||
|
||||
# update network
|
||||
if networks_update:
|
||||
self._update_network_update_port(obj, networks_update)
|
||||
|
||||
# Detach some existing interfaces
|
||||
if networks_delete:
|
||||
|
|
|
|||
|
|
@ -1195,9 +1195,91 @@ class TestNovaServerUpdate(base.SenlinTestCase):
|
|||
str(ex))
|
||||
cc.server_interface_delete.assert_called_once_with('port1', 'NOVA_ID')
|
||||
|
||||
def test_update_port(self):
|
||||
cc = mock.Mock()
|
||||
cc.server_get.return_value = mock.Mock(status=consts.VS_ACTIVE)
|
||||
nc = mock.Mock()
|
||||
net1 = mock.Mock(id='net1')
|
||||
nc.network_get.return_value = net1
|
||||
nc.port_find.return_value = mock.Mock(id='port3', status='DOWN')
|
||||
profile = server.ServerProfile('t', self.spec)
|
||||
profile.stop_timeout = 232
|
||||
profile._computeclient = cc
|
||||
profile._networkclient = nc
|
||||
validation_results = [
|
||||
{'network': 'net1_id', 'fixed_ip': 'ip1',
|
||||
'security_groups': ['sg1_id']},
|
||||
{'network': 'net1_id', 'fixed_ip': 'ip1',
|
||||
'security_groups': ['sg1_id', 'sg2_id']},
|
||||
{'network': 'net1_id', 'fixed_ip': 'ip1'}
|
||||
]
|
||||
mock_validate = self.patchobject(profile, '_validate_network',
|
||||
side_effect=validation_results)
|
||||
candidate_ports = [
|
||||
[{'id': 'port1_id', 'network_id': 'net1_id',
|
||||
'fixed_ips': [{'ip_address': 'ip1'}]}],
|
||||
[{'id': 'port2_id', 'network_id': 'net1_id',
|
||||
'fixed_ips': [{'ip_address': 'ip1'}]}],
|
||||
[{'id': 'port3_id', 'network_id': 'net1_id',
|
||||
'fixed_ips': [{'ip_address': 'ip1'}]}]
|
||||
]
|
||||
self.patchobject(profile, '_find_port_by_net_spec',
|
||||
side_effect=candidate_ports)
|
||||
|
||||
obj = mock.Mock(physical_id='NOVA_ID', data={'internal_ports': [
|
||||
{'id': 'port1', 'network_id': 'net1',
|
||||
'fixed_ips': [{'ip_address': 'ip1'}]},
|
||||
{'id': 'port2', 'network_id': 'net1', 'remove': True,
|
||||
'fixed_ips': [{'ip_address': 'ip-random2'}],
|
||||
'security_groups': ['default']},
|
||||
{'id': 'port3', 'network_id': 'net1', 'remove': True,
|
||||
'fixed_ips': [{'ip_address': 'ip3'}],
|
||||
'security_groups': ['default']}
|
||||
]})
|
||||
networks = [
|
||||
{'network': 'net1', 'port': None, 'fixed_ip': 'ip1',
|
||||
'security_groups': ['default'], 'floating_network': None,
|
||||
'floating_ip': None},
|
||||
{'network': 'net1', 'port': None, 'fixed_ip': 'ip1',
|
||||
'security_groups': ['default', 'blah'], 'floating_network': None,
|
||||
'floating_ip': None},
|
||||
{'network': 'net1', 'port': None, 'fixed_ip': 'ip1',
|
||||
'security_groups': None, 'floating_network': None,
|
||||
'floating_ip': None},
|
||||
]
|
||||
|
||||
res = profile._update_network_update_port(obj, networks)
|
||||
|
||||
self.assertIsNone(res)
|
||||
validation_calls = [
|
||||
mock.call(obj,
|
||||
{'network': 'net1', 'port': None, 'fixed_ip': 'ip1',
|
||||
'security_groups': ['default'],
|
||||
'floating_network': None, 'floating_ip': None},
|
||||
'update'),
|
||||
mock.call(obj,
|
||||
{'network': 'net1', 'port': None, 'fixed_ip': 'ip1',
|
||||
'security_groups': ['default', 'blah'],
|
||||
'floating_network': None, 'floating_ip': None},
|
||||
'update'),
|
||||
mock.call(obj,
|
||||
{'network': 'net1', 'port': None, 'fixed_ip': 'ip1',
|
||||
'security_groups': None, 'floating_network': None,
|
||||
'floating_ip': None},
|
||||
'update')
|
||||
]
|
||||
mock_validate.assert_has_calls(validation_calls)
|
||||
update_calls = [
|
||||
mock.call('port1_id', security_groups=['sg1_id']),
|
||||
mock.call('port2_id', security_groups=['sg1_id', 'sg2_id']),
|
||||
mock.call('port3_id', security_groups=[]),
|
||||
]
|
||||
nc.port_update.assert_has_calls(update_calls)
|
||||
|
||||
@mock.patch.object(server.ServerProfile, '_update_network_update_port')
|
||||
@mock.patch.object(server.ServerProfile, '_update_network_remove_port')
|
||||
@mock.patch.object(server.ServerProfile, '_update_network_add_port')
|
||||
def test_update_network(self, mock_create, mock_delete):
|
||||
def test_update_network(self, mock_create, mock_delete, mock_update):
|
||||
obj = mock.Mock(physical_id='FAKE_ID')
|
||||
|
||||
old_spec = copy.deepcopy(self.spec)
|
||||
|
|
@ -1205,6 +1287,11 @@ class TestNovaServerUpdate(base.SenlinTestCase):
|
|||
{'network': 'net1', 'fixed_ip': 'ip1'},
|
||||
{'network': 'net1'},
|
||||
{'port': 'port3'},
|
||||
# sg only changes:
|
||||
{'network': 'net3', 'fixed_ip': 'ip1'},
|
||||
{'network': 'net4', 'fixed_ip': 'ip1',
|
||||
'security_groups': ['blah']},
|
||||
{'port': 'port5', 'security_groups': ['default']},
|
||||
]
|
||||
profile = server.ServerProfile('t', old_spec)
|
||||
new_spec = copy.deepcopy(self.spec)
|
||||
|
|
@ -1212,6 +1299,12 @@ class TestNovaServerUpdate(base.SenlinTestCase):
|
|||
{'network': 'net1', 'fixed_ip': 'ip2'},
|
||||
{'network': 'net2'},
|
||||
{'port': 'port4'},
|
||||
# sg only changes:
|
||||
{'network': 'net3', 'fixed_ip': 'ip1',
|
||||
'security_groups': ['default']},
|
||||
{'network': 'net4', 'fixed_ip': 'ip1',
|
||||
'security_groups': ['default']},
|
||||
{'port': 'port5', 'security_groups': ['default', 'blah']},
|
||||
]
|
||||
new_profile = server.ServerProfile('t1', new_spec)
|
||||
|
||||
|
|
@ -1239,6 +1332,18 @@ class TestNovaServerUpdate(base.SenlinTestCase):
|
|||
'floating_ip': None, 'port': 'port3', 'security_groups': None}
|
||||
]
|
||||
mock_delete.assert_called_once_with(obj, networks_delete)
|
||||
networks_update = [
|
||||
{'network': 'net3', 'port': None, 'fixed_ip': 'ip1',
|
||||
'security_groups': ['default'], 'floating_network': None,
|
||||
'floating_ip': None},
|
||||
{'network': 'net4', 'port': None, 'fixed_ip': 'ip1',
|
||||
'security_groups': ['default'], 'floating_network': None,
|
||||
'floating_ip': None},
|
||||
{'network': None, 'port': 'port5', 'fixed_ip': None,
|
||||
'security_groups': ['default', 'blah'], 'floating_network': None,
|
||||
'floating_ip': None}
|
||||
]
|
||||
mock_update.assert_called_once_with(obj, networks_update)
|
||||
|
||||
@mock.patch.object(server.ServerProfile, '_update_password')
|
||||
@mock.patch.object(server.ServerProfile, '_check_password')
|
||||
|
|
|
|||
Loading…
Reference in New Issue