From a5598a52a9b992740a6ae164545279b4d2ee42e9 Mon Sep 17 00:00:00 2001 From: Andrew Melton Date: Wed, 18 Feb 2015 09:31:45 -0800 Subject: [PATCH] Allow pod delete to succeed when not found on bay If a pod is deleted from kubernetes outside of magnum, and then a pod-delete is issued through magnum, the delete will succeed but will leave the record in the database. This change raises an exception when the pod is not found, but catches it in the conductor and proceeds to delete the record from the database. Change-Id: I83a6b3e68eef434fb7d1b4cba92acb17abc6bc4d Closes-bug: #1412587 --- .../conductor/handlers/common/kube_utils.py | 10 ++- .../handlers/common/test_kube_utils.py | 67 +++++++++++++++++ magnum/tests/conductor/handlers/test_kube.py | 74 ++++++++++++++++++- 3 files changed, 148 insertions(+), 3 deletions(-) diff --git a/magnum/conductor/handlers/common/kube_utils.py b/magnum/conductor/handlers/common/kube_utils.py index 6b43d4f5c7..ef014d5595 100644 --- a/magnum/conductor/handlers/common/kube_utils.py +++ b/magnum/conductor/handlers/common/kube_utils.py @@ -14,6 +14,7 @@ import tempfile +from magnum.common import exception from magnum.openstack.common import log as logging from magnum.openstack.common import utils @@ -191,11 +192,16 @@ class KubeClient(object): try: out, err = utils.trycmd('kubectl', 'delete', 'pod', name, '-s', master_address,) - if err: - return False except Exception as e: LOG.error("Couldn't delete pod %s due to error %s" % (name, e)) return False + + if err: + if ('pod "%s" not found' % name) in err: + raise exception.PodNotFound(pod=name) + else: + return False + return True def pod_get(self, master_address, uuid): diff --git a/magnum/tests/conductor/handlers/common/test_kube_utils.py b/magnum/tests/conductor/handlers/common/test_kube_utils.py index a9453baa53..9df09d6cf6 100644 --- a/magnum/tests/conductor/handlers/common/test_kube_utils.py +++ b/magnum/tests/conductor/handlers/common/test_kube_utils.py @@ -12,6 +12,7 @@ # License for the specific language governing permissions and limitations # under the License. +from magnum.common import exception from magnum.conductor.handlers.common import kube_utils from magnum.tests import base @@ -142,3 +143,69 @@ class TestKubeUtils(base.BaseTestCase): mock_file.write.assert_called_once_with(expected_data) mock_k8s_update.assert_called_once_with(expected_master_address, expected_filename) + + +class KubeClientTestCase(base.TestCase): + def setUp(self): + super(KubeClientTestCase, self).setUp() + self.kube_client = kube_utils.KubeClient() + + @patch('magnum.openstack.common.utils.trycmd') + def test_pod_delete(self, mock_trycmd): + expected_master_address = 'master-address' + expected_pod_name = 'test-pod' + expected_command = [ + 'kubectl', 'delete', 'pod', expected_pod_name, + '-s', expected_master_address + ] + mock_trycmd.return_value = ("", "") + + result = self.kube_client.pod_delete(expected_master_address, + expected_pod_name) + self.assertTrue(result) + mock_trycmd.assert_called_once_with(*expected_command) + + @patch('magnum.openstack.common.utils.trycmd') + def test_pod_delete_failure_err_not_empty(self, mock_trycmd): + expected_master_address = 'master-address' + expected_pod_name = 'test-pod' + expected_command = [ + 'kubectl', 'delete', 'pod', expected_pod_name, + '-s', expected_master_address + ] + mock_trycmd.return_value = ("", "error") + + result = self.kube_client.pod_delete(expected_master_address, + expected_pod_name) + self.assertFalse(result) + mock_trycmd.assert_called_once_with(*expected_command) + + @patch('magnum.openstack.common.utils.trycmd') + def test_pod_delete_failure_exception(self, mock_trycmd): + expected_master_address = 'master-address' + expected_pod_name = 'test-pod' + expected_command = [ + 'kubectl', 'delete', 'pod', expected_pod_name, + '-s', expected_master_address + ] + mock_trycmd.side_effect = Exception() + + result = self.kube_client.pod_delete(expected_master_address, + expected_pod_name) + self.assertFalse(result) + mock_trycmd.assert_called_once_with(*expected_command) + + @patch('magnum.openstack.common.utils.trycmd') + def test_pod_delete_not_found(self, mock_trycmd): + expected_master_address = 'master-address' + expected_pod_name = 'test-pod' + expected_command = [ + 'kubectl', 'delete', 'pod', expected_pod_name, + '-s', expected_master_address + ] + mock_trycmd.return_value = ("", 'pod "test-pod" not found') + + self.assertRaises(exception.PodNotFound, self.kube_client.pod_delete, + expected_master_address, expected_pod_name) + + mock_trycmd.assert_called_once_with(*expected_command) diff --git a/magnum/tests/conductor/handlers/test_kube.py b/magnum/tests/conductor/handlers/test_kube.py index c40d695eb8..011708d447 100644 --- a/magnum/tests/conductor/handlers/test_kube.py +++ b/magnum/tests/conductor/handlers/test_kube.py @@ -28,7 +28,7 @@ cfg.CONF.import_opt('k8s_port', 'magnum.conductor.handlers.kube', group='kubernetes') -class TestKube(base.BaseTestCase): +class TestKube(base.TestCase): def setUp(self): super(TestKube, self).setUp() self.kube_handler = kube.Handler() @@ -146,6 +146,78 @@ class TestKube(base.BaseTestCase): self.kube_handler.pod_create({}, expected_pod) self.assertEqual('failed', expected_pod.status) + @patch('magnum.conductor.handlers.kube._object_has_stack') + @patch('magnum.conductor.handlers.kube._retrieve_k8s_master_url') + @patch('magnum.objects.Pod.get_by_uuid') + def test_pod_delete_with_success(self, + mock_pod_get_by_uuid, + mock_retrieve_k8s_master_url, + mock_object_has_stack): + expected_master_url = 'master_address' + mock_pod = mock.MagicMock() + mock_pod.name = 'test-pod' + mock_pod.uuid = 'test-uuid' + mock_pod_get_by_uuid.return_value = mock_pod + + mock_retrieve_k8s_master_url.return_value = expected_master_url + mock_object_has_stack.return_value = True + with patch.object(self.kube_handler, 'kube_cli') as mock_kube_cli: + mock_kube_cli.pod_delete.return_value = True + + self.kube_handler.pod_delete(self.context, mock_pod.uuid) + + mock_kube_cli.pod_delete.assert_called_once_with( + expected_master_url, mock_pod.name) + mock_pod.destroy.assert_called_once_with(self.context) + + @patch('magnum.conductor.handlers.kube._object_has_stack') + @patch('magnum.conductor.handlers.kube._retrieve_k8s_master_url') + @patch('magnum.objects.Pod.get_by_uuid') + def test_pod_delete_with_failure(self, + mock_pod_get_by_uuid, + mock_retrieve_k8s_master_url, + mock_object_has_stack): + expected_master_url = 'master_address' + mock_pod = mock.MagicMock() + mock_pod.name = 'test-pod' + mock_pod.uuid = 'test-uuid' + mock_pod_get_by_uuid.return_value = mock_pod + + mock_retrieve_k8s_master_url.return_value = expected_master_url + mock_object_has_stack.return_value = True + with patch.object(self.kube_handler, 'kube_cli') as mock_kube_cli: + mock_kube_cli.pod_delete.return_value = False + + self.kube_handler.pod_delete(self.context, mock_pod.uuid) + + mock_kube_cli.pod_delete.assert_called_once_with( + expected_master_url, mock_pod.name) + self.assertFalse(mock_pod.destroy.called) + + @patch('magnum.conductor.handlers.kube._object_has_stack') + @patch('magnum.conductor.handlers.kube._retrieve_k8s_master_url') + @patch('magnum.objects.Pod.get_by_uuid') + def test_pod_delete_succeeds_when_not_found(self, + mock_pod_get_by_uuid, + mock_retrieve_k8s_master_url, + mock_object_has_stack): + expected_master_url = 'master_address' + mock_pod = mock.MagicMock() + mock_pod.name = 'test-pod' + mock_pod.uuid = 'test-uuid' + mock_pod_get_by_uuid.return_value = mock_pod + + mock_retrieve_k8s_master_url.return_value = expected_master_url + mock_object_has_stack.return_value = True + with patch.object(self.kube_handler, 'kube_cli') as mock_kube_cli: + mock_kube_cli.pod_delete.return_value = True + + self.kube_handler.pod_delete(self.context, mock_pod.uuid) + + mock_kube_cli.pod_delete.assert_called_once_with( + expected_master_url, mock_pod.name) + mock_pod.destroy.assert_called_once_with(self.context) + @patch('magnum.conductor.handlers.kube._retrieve_k8s_master_url') def test_service_create_with_success(self, mock_retrieve_k8s_master_url):