From c2bf0d8b576c8ce2f293fd85bdc1877de8aafe96 Mon Sep 17 00:00:00 2001 From: poojajadhav Date: Mon, 30 Jan 2017 20:19:37 +0530 Subject: [PATCH] Boolean filters are not working for host list api If user tries to retrieve hosts along with filter set for on_maintenance or reserved as True, it always returns records with boolean filters set to False, irrespective of the value provided to boolean filters flag in the filter. This patch fixes boolean filter issue by converting value from string to boolean using oslo_utils.strutils method. Closes-Bug: #1658911 Change-Id: I5eed5c75f2b7613196dcf1686e5dd983f31c2a81 --- masakari/api/openstack/ha/hosts.py | 19 ++++- .../tests/unit/api/openstack/ha/test_hosts.py | 82 +++++++++++++++++++ 2 files changed, 99 insertions(+), 2 deletions(-) diff --git a/masakari/api/openstack/ha/hosts.py b/masakari/api/openstack/ha/hosts.py index 08417c7b..c6778460 100644 --- a/masakari/api/openstack/ha/hosts.py +++ b/masakari/api/openstack/ha/hosts.py @@ -15,6 +15,8 @@ """The Host API extension.""" +from oslo_utils import encodeutils +from oslo_utils import strutils from six.moves import http_client as http from webob import exc @@ -25,6 +27,7 @@ from masakari.api.openstack import wsgi from masakari.api import validation from masakari import exception from masakari.ha import api as host_api +from masakari.i18n import _ from masakari import objects ALIAS = "os-hosts" @@ -64,10 +67,22 @@ class HostsController(wsgi.Controller): 'control_attributes'] if 'on_maintenance' in req.params: - filters['on_maintenance'] = req.params['on_maintenance'] + try: + filters['on_maintenance'] = strutils.bool_from_string( + req.params['on_maintenance'], strict=True) + except ValueError as ex: + msg = _("Invalid value for on_maintenance: " + "%s") % encodeutils.exception_to_unicode(ex) + raise exc.HTTPBadRequest(explanation=msg) if 'reserved' in req.params: - filters['reserved'] = req.params['reserved'] + try: + filters['reserved'] = strutils.bool_from_string( + req.params['reserved'], strict=True) + except ValueError as ex: + msg = _("Invalid value for reserved: " + "%s") % encodeutils.exception_to_unicode(ex) + raise exc.HTTPBadRequest(explanation=msg) hosts = self.api.get_all(context, filters=filters, sort_keys=sort_keys, sort_dirs=sort_dirs, diff --git a/masakari/tests/unit/api/openstack/ha/test_hosts.py b/masakari/tests/unit/api/openstack/ha/test_hosts.py index 020965b1..f85fb358 100644 --- a/masakari/tests/unit/api/openstack/ha/test_hosts.py +++ b/masakari/tests/unit/api/openstack/ha/test_hosts.py @@ -100,6 +100,88 @@ class HostTestCase(test.TestCase): result = result['hosts'] self._assert_host_data(HOST_LIST, _make_hosts_list(result)) + @mock.patch.object(segment_obj.FailoverSegment, 'get_by_uuid') + @mock.patch.object(ha_api.HostAPI, 'get_all') + def test_index_valid_on_maintenance(self, mock_get_all, mock_segment): + host_list = [{"name": "host_1", "id": "1", "on_maintenance": True}, + {"name": "host_2", "id": "2", "on_maintenance": True}] + mock_get_all.return_value = host_list + for parameter in ['1', 't', 'true', 'on', 'y', 'yes']: + req = fakes.HTTPRequest.blank( + '/v1/segments/%s/hosts?on_maintenance=''%s' % ( + uuidsentinel.fake_segment1, parameter), + use_admin_context=True) + result = self.controller.index(req, uuidsentinel.fake_segment1) + self.assertIn('hosts', result) + self.assertEqual(len(host_list), len(result['hosts'])) + for host in result['hosts']: + self.assertTrue(host['on_maintenance']) + + host_list = [{"name": "host_1", "id": "1", "on_maintenance": False}, + {"name": "host_2", "id": "2", "on_maintenance": False}] + mock_get_all.return_value = host_list + for parameter in ['0', 'f', 'false', 'off', 'n', 'no']: + req = fakes.HTTPRequest.blank( + '/v1/segments/%s/hosts?on_maintenance=''%s' % ( + uuidsentinel.fake_segment1, parameter), + use_admin_context=True) + result = self.controller.index(req, uuidsentinel.fake_segment1) + self.assertIn('hosts', result) + self.assertEqual(len(host_list), len(result['hosts'])) + for host in result['hosts']: + self.assertFalse(host['on_maintenance']) + + @mock.patch.object(segment_obj.FailoverSegment, 'get_by_uuid', + return_value=mock.Mock()) + def test_index_invalid_on_maintenance(self, mock_segment): + + req = fakes.HTTPRequest.blank('/v1/segments/%s/hosts?on_maintenance=' + 'abcd' % uuidsentinel.fake_segment1, + use_admin_context=True) + self.assertRaises(exc.HTTPBadRequest, self.controller.index, req, + uuidsentinel.fake_segment1) + + @mock.patch.object(segment_obj.FailoverSegment, 'get_by_uuid') + @mock.patch.object(ha_api.HostAPI, 'get_all') + def test_index_valid_reserved(self, mock_get_all, mock_segment): + host_list = [{"name": "host_1", "id": "1", "reserved": True}, + {"name": "host_2", "id": "2", "reserved": True}] + mock_get_all.return_value = host_list + for parameter in ['1', 't', 'true', 'on', 'y', 'yes']: + req = fakes.HTTPRequest.blank( + '/v1/segments/%s/hosts?reserved=''%s' % ( + uuidsentinel.fake_segment1, parameter + ), use_admin_context=True) + result = self.controller.index(req, uuidsentinel.fake_segment1) + self.assertIn('hosts', result) + self.assertEqual(len(host_list), len(result['hosts'])) + for host in result['hosts']: + self.assertTrue(host['reserved']) + + host_list = [{"name": "host_1", "id": "1", "reserved": False}, + {"name": "host_2", "id": "2", "reserved": False}] + mock_get_all.return_value = host_list + for parameter in ['0', 'f', 'false', 'off', 'n', 'no']: + req = fakes.HTTPRequest.blank( + '/v1/segments/%s/hosts?reserved=''%s' % ( + uuidsentinel.fake_segment1, parameter), + use_admin_context=True) + result = self.controller.index(req, uuidsentinel.fake_segment1) + self.assertIn('hosts', result) + self.assertEqual(len(host_list), len(result['hosts'])) + for host in result['hosts']: + self.assertFalse(host['reserved']) + + @mock.patch.object(segment_obj.FailoverSegment, 'get_by_uuid', + return_value=mock.Mock()) + def test_index_invalid_reserved(self, mock_segment): + + req = fakes.HTTPRequest.blank('/v1/segments/%s/hosts?reserved=' + 'abcd' % uuidsentinel.fake_segment1, + use_admin_context=True) + self.assertRaises(exc.HTTPBadRequest, self.controller.index, req, + uuidsentinel.fake_segment1) + @mock.patch.object(segment_obj.FailoverSegment, 'get_by_uuid') @mock.patch.object(ha_api.HostAPI, 'get_all') def test_index_marker_not_found(self, mock_get_all, mock_segment):