From 705d2a0b7b78813fb84636ac262030bc7dd6be08 Mon Sep 17 00:00:00 2001 From: ricolin Date: Wed, 7 Feb 2018 14:14:27 +0800 Subject: [PATCH] Check NotFound with pool property in pool member We use pool property directly when create/update/delete/show pool member. Which will try to translate and raise value error when it's actually pool not found. In cases like pool can not be found, we will raise NotFound exception instead a value error. Story: #1747836 Task: #16037 Change-Id: Ibc8f5d125436f0f8c9ea5a62d5be4e4f8b3c4401 --- .../openstack/octavia/pool_member.py | 18 +++-- .../openstack/octavia/test_pool_member.py | 72 +++++++++++++++++++ 2 files changed, 85 insertions(+), 5 deletions(-) diff --git a/heat/engine/resources/openstack/octavia/pool_member.py b/heat/engine/resources/openstack/octavia/pool_member.py index e4d29e8e0d..b335e335c6 100644 --- a/heat/engine/resources/openstack/octavia/pool_member.py +++ b/heat/engine/resources/openstack/octavia/pool_member.py @@ -131,20 +131,28 @@ class PoolMember(octavia_base.OctaviaBase): def _resource_create(self, properties): return self.client().member_create( - self.properties[self.POOL], json={'member': properties})['member'] + self._get_pool_prop(), json={'member': properties})['member'] def _resource_update(self, prop_diff): - self.client().member_set(self.properties[self.POOL], + self.client().member_set(self._get_pool_prop(), self.resource_id, json={'member': prop_diff}) + def _get_pool_prop(self): + try: + return self.properties[self.POOL] + except ValueError: + if self.client_plugin().get_pool(self.properties.data[self.POOL]): + raise + def _resource_delete(self): - self.client().member_delete(self.properties[self.POOL], - self.resource_id) + pool = self._get_pool_prop() + if pool: + self.client().member_delete(pool, self.resource_id) def _show_resource(self): return self.client().member_show( - self.properties[self.POOL], self.resource_id) + self._get_pool_prop(), self.resource_id) def resource_mapping(): diff --git a/heat/tests/openstack/octavia/test_pool_member.py b/heat/tests/openstack/octavia/test_pool_member.py index 93035c18d7..9a0a2a9e2b 100644 --- a/heat/tests/openstack/octavia/test_pool_member.py +++ b/heat/tests/openstack/octavia/test_pool_member.py @@ -17,6 +17,7 @@ from neutronclient.neutron import v2_0 as neutronV20 from osc_lib import exceptions from heat.common import template_format +from heat.engine import properties from heat.engine.resources.openstack.octavia import pool_member from heat.tests import common from heat.tests.openstack.octavia import inline_templates @@ -155,6 +156,48 @@ class PoolMemberTest(common.HeatTestCase): '1234') self.assertFalse(self.member._delete_called) + def _prepare_delete_with_value_error(self): + self._create_stack() + self.member.resource_id_set('1234') + self.m_gpv = self.patchobject(properties.Properties, + '_get_property_value', + side_effect=ValueError) + + def test_delete_value_error_with_not_found(self): + self._prepare_delete_with_value_error() + m_get_pool = mock.Mock(side_effect=exceptions.NotFound(404)) + self.member.client_plugin().get_pool = m_get_pool + + self.member.handle_delete() + + self.assertTrue(self.member.check_delete_complete(None)) + m_get_pool.assert_called_once_with(123) + self.assertFalse(self.member._delete_called) + self.assertEqual(0, self.octavia_client.member_delete.call_count) + self.m_gpv.assert_called_once_with('pool') + + def test_delete_value_error_with_pool_found(self): + self._prepare_delete_with_value_error() + self.member.client_plugin().get_pool = mock.Mock(return_value='123') + + self.member.handle_delete() + + self.assertRaises(ValueError, self.member.check_delete_complete, None) + self.assertFalse(self.member._delete_called) + self.assertEqual(0, self.octavia_client.member_delete.call_count) + self.m_gpv.assert_called_once_with('pool') + + def test_delete_value_error_with_pool_error(self): + self._prepare_delete_with_value_error() + self.member.client_plugin().get_pool = mock.Mock(side_effect=KeyError) + + self.member.handle_delete() + + self.assertRaises(KeyError, self.member.check_delete_complete, None) + self.assertFalse(self.member._delete_called) + self.assertEqual(0, self.octavia_client.member_delete.call_count) + self.m_gpv.assert_called_once_with('pool') + def test_delete_failed(self): self._create_stack() self.member.resource_id_set('1234') @@ -165,3 +208,32 @@ class PoolMemberTest(common.HeatTestCase): self.assertRaises(exceptions.Unauthorized, self.member.check_delete_complete, None) + + def _prepare_get_pool_prop(self): + self._create_stack() + self.member.resource_id_set('1234') + self.m_prop = self.patchobject( + properties.Properties, '_get_property_value') + + def test_get_pool_prop(self): + self._prepare_get_pool_prop() + self.member._get_pool_prop() + self.m_prop.assert_called_once_with('pool') + + def _get_pool_prop_with_value_error(self, side_effect=[]): + self._prepare_get_pool_prop() + self.m_prop.side_effect = ValueError + self.member.client_plugin().get_pool = mock.Mock( + side_effect=side_effect) + + def test_get_pool_prop_value_error_and_raise_exception(self): + self._get_pool_prop_with_value_error( + side_effect=exceptions.NotFound(404)) + + self.assertRaises( + exceptions.NotFound, self.member._get_pool_prop) + + def test_get_pool_prop_value_error_and_raise(self): + self._get_pool_prop_with_value_error(side_effect=['foo']) + + self.assertRaises(ValueError, self.member._get_pool_prop)