Merge "Fix duplicating network ports on node recovery"

This commit is contained in:
Zuul 2019-02-12 02:29:16 +00:00 committed by Gerrit Code Review
commit 0e35bd5d48
6 changed files with 98 additions and 52 deletions

View File

@ -1306,7 +1306,6 @@ def action_acquire(context, action_id, owner, timestamp):
return None
if action.status != consts.ACTION_READY:
LOG.warning('The action is not executable: %s', action.status)
return None
action.owner = owner
action.start_time = timestamp

View File

@ -39,7 +39,7 @@ def parse_exception(ex):
"""Parse exception code and yield useful information."""
code = 500
LOG.exception(ex)
LOG.error(six.text_type(ex))
if isinstance(ex, sdk_exc.HttpException):
# some exceptions don't contain status_code
if hasattr(ex, "status_code") and ex.status_code is not None:
@ -98,7 +98,6 @@ def translate_exception(func):
try:
return func(driver, *args, **kwargs)
except Exception as ex:
LOG.exception(ex)
raise parse_exception(ex)
return invoke_with_catch

View File

@ -537,8 +537,7 @@ class Profile(object):
extra_params = options.get('params', {})
fence_compute = extra_params.get('fence_compute', False)
try:
self.do_delete(obj, force=fence_compute, timeout=delete_timeout,
delete_ports_on_failure=force_recreate)
self.do_delete(obj, force=fence_compute, timeout=delete_timeout)
except exc.EResourceDeletion as ex:
if force_recreate:
# log error and continue on to creating the node

View File

@ -860,6 +860,7 @@ class ServerProfile(base.Profile):
kwargs['user_data'] = encodeutils.safe_decode(base64.b64encode(ud))
networks = self.properties[self.NETWORKS]
ports = None
if networks is not None:
ports = self._create_ports_from_properties(
obj, networks, 'create')
@ -892,6 +893,8 @@ class ServerProfile(base.Profile):
except exc.InternalError as ex:
if server and server.id:
resource_id = server.id
if ports:
self._delete_ports(obj, ports)
raise exc.EResourceCreation(type='server',
message=six.text_type(ex),
resource_id=resource_id)
@ -907,39 +910,29 @@ class ServerProfile(base.Profile):
:raises: `EResourceDeletion` if interaction with compute service fails.
"""
server_id = obj.physical_id
if not server_id:
return True
ignore_missing = params.get('ignore_missing', True)
internal_ports = obj.data.get('internal_ports', [])
force = params.get('force', False)
timeout = params.get('timeout', None)
delete_ports_on_failure = params.get('delete_ports_on_failure', False)
try:
driver = self.compute(obj)
if force:
driver.server_force_delete(server_id, ignore_missing)
else:
driver.server_delete(server_id, ignore_missing)
if server_id:
driver = self.compute(obj)
if force:
driver.server_force_delete(server_id, ignore_missing)
else:
driver.server_delete(server_id, ignore_missing)
driver.wait_for_server_delete(server_id, timeout)
except exc.InternalError as ex:
raise exc.EResourceDeletion(type='server', id=server_id,
message=six.text_type(ex))
try:
driver.wait_for_server_delete(server_id, timeout)
except exc.InternalError as ex:
if delete_ports_on_failure and internal_ports:
del_ports_ex = self._delete_ports(obj, internal_ports)
if del_ports_ex:
raise del_ports_ex
raise exc.EResourceDeletion(type='server', id=server_id,
message=six.text_type(ex))
if internal_ports:
ex = self._delete_ports(obj, internal_ports)
if ex:
raise ex
finally:
if internal_ports:
ex = self._delete_ports(obj, internal_ports)
if ex:
raise exc.EResourceDeletion(type='server', d=server_id,
message=six.text_type(ex))
return True
def _check_server_name(self, obj, profile):

View File

@ -571,7 +571,8 @@ class TestNovaServerBasic(base.SenlinTestCase):
mock_zone_info.assert_called_once_with(node_obj, fake_server)
self.assertEqual('FAKE_ID', server_id)
def test_do_create_wait_server_timeout(self):
@mock.patch.object(node_ob.Node, 'update')
def test_do_create_wait_server_timeout(self, mock_node_obj):
cc = mock.Mock()
nc = mock.Mock()
profile = server.ServerProfile('t', self.spec)
@ -599,9 +600,12 @@ class TestNovaServerBasic(base.SenlinTestCase):
self.assertEqual('FAKE_ID', ex.resource_id)
self.assertEqual('Failed in creating server: TIMEOUT.',
six.text_type(ex))
mock_node_obj.assert_called_once_with(mock.ANY, node_obj.id,
{'data': node_obj.data})
cc.wait_for_server.assert_called_once_with('FAKE_ID')
def test_do_create_failed(self):
@mock.patch.object(node_ob.Node, 'update')
def test_do_create_failed(self, mock_node_obj):
cc = mock.Mock()
profile = server.ServerProfile('t', self.spec)
profile._computeclient = cc
@ -625,6 +629,8 @@ class TestNovaServerBasic(base.SenlinTestCase):
node_obj)
# assertions
mock_node_obj.assert_called_once_with(mock.ANY, node_obj.id,
{'data': node_obj.data})
self.assertEqual('Failed in creating server: creation failed.',
six.text_type(ex))
self.assertIsNone(ex.resource_id)
@ -654,6 +660,7 @@ class TestNovaServerBasic(base.SenlinTestCase):
profile._computeclient = cc
test_server = mock.Mock(physical_id=None)
test_server.data = {}
# do it
res = profile.do_delete(test_server)
@ -663,6 +670,37 @@ class TestNovaServerBasic(base.SenlinTestCase):
self.assertFalse(cc.server_delete.called)
self.assertFalse(cc.wait_for_server_delete.called)
@mock.patch.object(node_ob.Node, 'update')
def test_do_delete_no_physical_id_with_internal_ports(self, mock_node_obj):
profile = server.ServerProfile('t', self.spec)
cc = mock.Mock()
nc = mock.Mock()
nc.port_delete.return_value = None
nc.floatingip_delete.return_value = None
profile._computeclient = cc
profile._networkclient = nc
test_server = mock.Mock(physical_id=None)
test_server.data = {'internal_ports': [{
'floating': {
'remove': True,
'id': 'FAKE_FLOATING_ID',
},
'id': 'FAKE_PORT_ID',
'remove': True
}]}
# do it
res = profile.do_delete(test_server)
# assertions
self.assertTrue(res)
mock_node_obj.assert_called_once_with(
mock.ANY, test_server.id, {'data': {'internal_ports': []}})
self.assertFalse(cc.server_delete.called)
self.assertFalse(cc.wait_for_server_delete.called)
@mock.patch.object(node_ob.Node, 'update')
def test_do_delete_ports_ok(self, mock_node_obj):
profile = server.ServerProfile('t', self.spec)
@ -711,7 +749,8 @@ class TestNovaServerBasic(base.SenlinTestCase):
cc.server_force_delete.assert_called_once_with('FAKE_ID', False)
cc.wait_for_server_delete.assert_called_once_with('FAKE_ID', None)
def test_do_delete_with_delete_failure(self):
@mock.patch.object(node_ob.Node, 'update')
def test_do_delete_with_delete_failure(self, mock_node_obj):
cc = mock.Mock()
nc = mock.Mock()
profile = server.ServerProfile('t', self.spec)
@ -721,18 +760,29 @@ class TestNovaServerBasic(base.SenlinTestCase):
err = exc.InternalError(code=500, message='Nova Error')
cc.server_delete.side_effect = err
obj = mock.Mock(physical_id='FAKE_ID')
obj.data = {'internal_ports': [{
'floating': {
'remove': True,
'id': 'FAKE_FLOATING_ID',
},
'id': 'FAKE_PORT_ID',
'remove': True
}]}
# do it
ex = self.assertRaises(exc.EResourceDeletion,
profile.do_delete, obj)
mock_node_obj.assert_called_once_with(mock.ANY, obj.id,
{'data': obj.data})
self.assertEqual("Failed in deleting server 'FAKE_ID': "
"Nova Error.", six.text_type(ex))
cc.server_delete.assert_called_once_with('FAKE_ID', True)
self.assertEqual(0, cc.wait_for_server_delete.call_count)
nc.port_delete.assert_not_called()
nc.port_delete.assert_called_once_with('FAKE_PORT_ID')
def test_do_delete_with_force_delete_failure(self):
@mock.patch.object(node_ob.Node, 'update')
def test_do_delete_with_force_delete_failure(self, mock_node_obj):
cc = mock.Mock()
nc = mock.Mock()
profile = server.ServerProfile('t', self.spec)
@ -742,40 +792,53 @@ class TestNovaServerBasic(base.SenlinTestCase):
err = exc.InternalError(code=500, message='Nova Error')
cc.server_force_delete.side_effect = err
obj = mock.Mock(physical_id='FAKE_ID')
obj.data = {}
# do it
ex = self.assertRaises(exc.EResourceDeletion,
profile.do_delete, obj, force=True)
mock_node_obj.assert_not_called()
self.assertEqual("Failed in deleting server 'FAKE_ID': "
"Nova Error.", six.text_type(ex))
cc.server_force_delete.assert_called_once_with('FAKE_ID', True)
self.assertEqual(0, cc.wait_for_server_delete.call_count)
nc.port_delete.assert_not_called()
def test_do_delete_wait_for_server_timeout(self):
@mock.patch.object(node_ob.Node, 'update')
def test_do_delete_wait_for_server_timeout(self, mock_node_obj):
cc = mock.Mock()
nc = mock.Mock()
profile = server.ServerProfile('t', self.spec)
profile._computeclient = cc
profile._networkclient = nc
obj = mock.Mock(physical_id='FAKE_ID')
err = exc.InternalError(code=500, message='TIMEOUT')
cc.wait_for_server_delete.side_effect = err
obj = mock.Mock(physical_id='FAKE_ID')
obj.data = {'internal_ports': [{
'floating': {
'remove': True,
'id': 'FAKE_FLOATING_ID',
},
'id': 'FAKE_PORT_ID',
'remove': True
}]}
# do it
ex = self.assertRaises(exc.EResourceDeletion,
profile.do_delete, obj, timeout=20)
mock_node_obj.assert_called_once_with(mock.ANY, obj.id,
{'data': obj.data})
self.assertEqual("Failed in deleting server 'FAKE_ID': TIMEOUT.",
six.text_type(ex))
cc.server_delete.assert_called_once_with('FAKE_ID', True)
cc.wait_for_server_delete.assert_called_once_with('FAKE_ID', 20)
nc.port_delete.assert_not_called()
nc.port_delete.assert_called_once_with('FAKE_PORT_ID')
@mock.patch.object(node_ob.Node, 'update')
def test_do_delete_wait_for_server_timeout_delete_ports_on_failure(
def test_do_delete_wait_for_server_timeout_delete_ports(
self, mock_node_obj):
cc = mock.Mock()
nc = mock.Mock()
@ -801,8 +864,7 @@ class TestNovaServerBasic(base.SenlinTestCase):
# do it
ex = self.assertRaises(exc.EResourceDeletion,
profile.do_delete, test_server, timeout=20,
delete_ports_on_failure=True)
profile.do_delete, test_server, timeout=20)
self.assertEqual("Failed in deleting server 'FAKE_ID': TIMEOUT.",
six.text_type(ex))
@ -832,8 +894,7 @@ class TestNovaServerBasic(base.SenlinTestCase):
# do it
ex = self.assertRaises(exc.EResourceDeletion,
profile.do_delete, test_server, timeout=20,
delete_ports_on_failure=True)
profile.do_delete, test_server, timeout=20)
self.assertEqual("Failed in deleting server 'FAKE_ID': TIMEOUT.",
six.text_type(ex))
@ -1452,8 +1513,7 @@ class TestNovaServerBasic(base.SenlinTestCase):
self.assertTrue(res)
mock_delete.assert_called_once_with(node_obj, force=False,
timeout=None,
delete_ports_on_failure=None)
timeout=None)
mock_create.assert_called_once_with(node_obj)
@mock.patch.object(server.ServerProfile, 'handle_rebuild')

View File

@ -855,8 +855,7 @@ class TestProfileBase(base.SenlinTestCase):
self.assertTrue(res)
profile.do_delete.assert_called_once_with(obj, force=True,
timeout=None,
delete_ports_on_failure=None)
timeout=None)
profile.do_create.assert_called_once_with(obj)
def test_do_recover_with_delete_timeout(self):
@ -869,8 +868,7 @@ class TestProfileBase(base.SenlinTestCase):
self.assertTrue(res)
profile.do_delete.assert_called_once_with(obj, force=False,
timeout=5,
delete_ports_on_failure=None)
timeout=5)
profile.do_create.assert_called_once_with(obj)
def test_do_recover_with_force_recreate(self):
@ -883,8 +881,7 @@ class TestProfileBase(base.SenlinTestCase):
self.assertTrue(res)
profile.do_delete.assert_called_once_with(obj, force=False,
timeout=None,
delete_ports_on_failure=True)
timeout=None)
profile.do_create.assert_called_once_with(obj)
def test_do_recover_with_force_recreate_failed_delete(self):
@ -898,8 +895,7 @@ class TestProfileBase(base.SenlinTestCase):
res = profile.do_recover(obj, ignore_missing=True, force_recreate=True)
self.assertTrue(res)
profile.do_delete.assert_called_once_with(obj, force=False,
timeout=None,
delete_ports_on_failure=True)
timeout=None)
profile.do_create.assert_called_once_with(obj)
def test_do_recover_with_false_force_recreate_failed_delete(self):