From e67b53211019539ee10173ab1310a8758163922a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vincent=20Fran=C3=A7oise?= Date: Tue, 12 Apr 2016 10:26:06 +0200 Subject: [PATCH] Refactored Strategy selector to select from DB In this changeset, I refactored the strategy selector to now look into the Watcher DB instead of looking into the configuration file. Partially Implements: blueprint get-goal-from-strategy Change-Id: I2bcb63542f6237f26796a3e5a781c8b62820cf6f --- devstack/lib/watcher | 2 +- watcher/common/exception.py | 6 +- .../strategy/context/default.py | 19 +++--- .../strategy/selection/base.py | 3 +- .../strategy/selection/default.py | 49 +++++++++++--- .../audit/test_default_audit_handler.py | 5 +- .../strategy/context/test_strategy_context.py | 16 ++--- .../selector/test_strategy_selector.py | 67 ++++++++++++------- .../tests/api/admin/test_audit_template.py | 2 +- .../scenario/test_execute_basic_optim.py | 2 +- 10 files changed, 111 insertions(+), 60 deletions(-) diff --git a/devstack/lib/watcher b/devstack/lib/watcher index afda8c0f7..68529c8af 100644 --- a/devstack/lib/watcher +++ b/devstack/lib/watcher @@ -123,7 +123,7 @@ function create_watcher_conf { iniset $WATCHER_CONF oslo_messaging_rabbit rabbit_password $RABBIT_PASSWORD iniset $WATCHER_CONF oslo_messaging_rabbit rabbit_host $RABBIT_HOST - iniset $WATCHER_CONF watcher_goals goals "DUMMY:dummy,BASIC_CONSOLIDATION:basic" + iniset $WATCHER_CONF watcher_goals goals "DUMMY:dummy,SERVER_CONSOLIDATION:basic" configure_auth_token_middleware $WATCHER_CONF watcher $WATCHER_AUTH_CACHE_DIR configure_auth_token_middleware $WATCHER_CONF watcher $WATCHER_AUTH_CACHE_DIR "watcher_clients_auth" diff --git a/watcher/common/exception.py b/watcher/common/exception.py index 46323ed09..a3b497b22 100644 --- a/watcher/common/exception.py +++ b/watcher/common/exception.py @@ -287,7 +287,11 @@ class MetricCollectorNotDefined(WatcherException): class ClusterStateNotDefined(WatcherException): - msg_fmt = _("the cluster state is not defined") + msg_fmt = _("The cluster state is not defined") + + +class NoAvailableStrategyForGoal(WatcherException): + msg_fmt = _("No strategy could be found to achieve the '%(goal)s' goal.") # Model diff --git a/watcher/decision_engine/strategy/context/default.py b/watcher/decision_engine/strategy/context/default.py index 90cee30b1..174693cf8 100644 --- a/watcher/decision_engine/strategy/context/default.py +++ b/watcher/decision_engine/strategy/context/default.py @@ -30,26 +30,20 @@ class DefaultStrategyContext(base.BaseStrategyContext): def __init__(self): super(DefaultStrategyContext, self).__init__() LOG.debug("Initializing Strategy Context") - self._strategy_selector = default.DefaultStrategySelector() self._collector_manager = manager.CollectorManager() @property def collector(self): return self._collector_manager - @property - def strategy_selector(self): - return self._strategy_selector - def execute_strategy(self, audit_uuid, request_context): audit = objects.Audit.get_by_uuid(request_context, audit_uuid) # Retrieve the Audit Template - audit_template = objects.\ - AuditTemplate.get_by_id(request_context, audit.audit_template_id) + audit_template = objects.AuditTemplate.get_by_id( + request_context, audit.audit_template_id) osc = clients.OpenStackClients() - # todo(jed) retrieve in audit_template parameters (threshold,...) # todo(jed) create ActionPlan collector_manager = self.collector.get_cluster_model_collector(osc=osc) @@ -57,8 +51,13 @@ class DefaultStrategyContext(base.BaseStrategyContext): # todo(jed) remove call to get_latest_cluster_data_model cluster_data_model = collector_manager.get_latest_cluster_data_model() - selected_strategy = self.strategy_selector.define_from_goal( - audit_template.goal, osc=osc) + strategy_selector = default.DefaultStrategySelector( + goal_name=objects.Goal.get_by_name( + request_context, audit_template.goal).name, + strategy_name=None, + osc=osc) + + selected_strategy = strategy_selector.select() # todo(jed) add parameters and remove cluster_data_model return selected_strategy.execute(cluster_data_model) diff --git a/watcher/decision_engine/strategy/selection/base.py b/watcher/decision_engine/strategy/selection/base.py index f0d74f9b4..7bf949029 100644 --- a/watcher/decision_engine/strategy/selection/base.py +++ b/watcher/decision_engine/strategy/selection/base.py @@ -22,6 +22,7 @@ import six @six.add_metaclass(abc.ABCMeta) class BaseSelector(object): + @abc.abstractmethod - def define_from_goal(self, goal_name): + def select(self): raise NotImplementedError() diff --git a/watcher/decision_engine/strategy/selection/default.py b/watcher/decision_engine/strategy/selection/default.py index d81a03891..bb21d30a3 100644 --- a/watcher/decision_engine/strategy/selection/default.py +++ b/watcher/decision_engine/strategy/selection/default.py @@ -44,19 +44,48 @@ CONF.register_opts(WATCHER_GOALS_OPTS, goals_opt_group) class DefaultStrategySelector(base.BaseSelector): - def __init__(self): + def __init__(self, goal_name, strategy_name=None, osc=None): + """Default strategy selector + + :param goal_name: Name of the goal + :param strategy_name: Name of the strategy + :param osc: an OpenStackClients instance + """ super(DefaultStrategySelector, self).__init__() + self.goal_name = goal_name + self.strategy_name = strategy_name + self.osc = osc self.strategy_loader = default.DefaultStrategyLoader() - def define_from_goal(self, goal_name, osc=None): - """:param osc: an OpenStackClients instance""" + def select(self): + """Selects a strategy + + :raises: :py:class:`~.LoadingError` if it failed to load a strategy + :returns: A :py:class:`~.BaseStrategy` instance + """ strategy_to_load = None try: - strategy_to_load = CONF.watcher_goals.goals[goal_name] - return self.strategy_loader.load(strategy_to_load, osc=osc) - except KeyError as exc: + if self.strategy_name: + strategy_to_load = self.strategy_name + else: + available_strategies = self.strategy_loader.list_available() + available_strategies_for_goal = list( + key for key, strat in available_strategies.items() + if strat.get_goal_name() == self.goal_name) + + if not available_strategies_for_goal: + raise exception.NoAvailableStrategyForGoal( + goal=self.goal_name) + + # TODO(v-francoise): We should do some more work here to select + # a strategy out of a given goal instead of just choosing the + # 1st one + strategy_to_load = available_strategies_for_goal[0] + return self.strategy_loader.load(strategy_to_load, osc=self.osc) + except exception.NoAvailableStrategyForGoal: + raise + except Exception as exc: LOG.exception(exc) - raise exception.WatcherException( - _("Incorrect mapping: could not find " - "associated strategy for '%s'") % goal_name - ) + raise exception.LoadingError( + _("Could not load any strategy for goal %(goal)s"), + goal=self.goal_name) diff --git a/watcher/tests/decision_engine/audit/test_default_audit_handler.py b/watcher/tests/decision_engine/audit/test_default_audit_handler.py index 2f1c18d00..4ba20aa4c 100644 --- a/watcher/tests/decision_engine/audit/test_default_audit_handler.py +++ b/watcher/tests/decision_engine/audit/test_default_audit_handler.py @@ -28,11 +28,12 @@ from watcher.tests.objects import utils as obj_utils class TestDefaultAuditHandler(base.DbTestCase): def setUp(self): super(TestDefaultAuditHandler, self).setUp() - self.audit_template = obj_utils.create_test_audit_template( + obj_utils.create_test_goal(self.context, id=1, name="DUMMY") + audit_template = obj_utils.create_test_audit_template( self.context) self.audit = obj_utils.create_test_audit( self.context, - audit_template_id=self.audit_template.id) + audit_template_id=audit_template.id) @mock.patch.object(manager.CollectorManager, "get_cluster_model_collector") def test_trigger_audit_without_errors(self, mock_collector): diff --git a/watcher/tests/decision_engine/strategy/context/test_strategy_context.py b/watcher/tests/decision_engine/strategy/context/test_strategy_context.py index 1536cda75..bdfd67c1d 100644 --- a/watcher/tests/decision_engine/strategy/context/test_strategy_context.py +++ b/watcher/tests/decision_engine/strategy/context/test_strategy_context.py @@ -31,19 +31,19 @@ from watcher.tests.objects import utils as obj_utils class TestStrategyContext(base.DbTestCase): def setUp(self): super(TestStrategyContext, self).setUp() - self.audit_template = obj_utils. \ - create_test_audit_template(self.context) - self.audit = obj_utils. \ - create_test_audit(self.context, - audit_template_id=self.audit_template.id) + obj_utils.create_test_goal(self.context, id=1, name="DUMMY") + audit_template = obj_utils.create_test_audit_template( + self.context) + self.audit = obj_utils.create_test_audit( + self.context, audit_template_id=audit_template.id) strategy_context = DefaultStrategyContext() - @mock.patch.object(DefaultStrategySelector, 'define_from_goal') + @mock.patch.object(DefaultStrategySelector, 'select') @mock.patch.object(CollectorManager, "get_cluster_model_collector", mock.Mock()) def test_execute_strategy(self, mock_call): mock_call.return_value = DummyStrategy() - solution = self.strategy_context.execute_strategy(self.audit.uuid, - self.context) + solution = self.strategy_context.execute_strategy( + self.audit.uuid, self.context) self.assertIsInstance(solution, DefaultSolution) diff --git a/watcher/tests/decision_engine/strategy/selector/test_strategy_selector.py b/watcher/tests/decision_engine/strategy/selector/test_strategy_selector.py index 3fc756428..0875f8786 100644 --- a/watcher/tests/decision_engine/strategy/selector/test_strategy_selector.py +++ b/watcher/tests/decision_engine/strategy/selector/test_strategy_selector.py @@ -13,37 +13,54 @@ # implied. # See the License for the specific language governing permissions and # limitations under the License. -from mock import patch + +import mock from oslo_config import cfg -from watcher.common.exception import WatcherException -from watcher.decision_engine.strategy.loading.default import \ - DefaultStrategyLoader -from watcher.decision_engine.strategy.selection.default import \ - DefaultStrategySelector -from watcher.tests.base import TestCase +from watcher.common import exception +from watcher.decision_engine.strategy.loading import default as default_loader +from watcher.decision_engine.strategy.selection import ( + default as default_selector) +from watcher.decision_engine.strategy import strategies +from watcher.tests import base CONF = cfg.CONF -class TestStrategySelector(TestCase): - strategy_selector = DefaultStrategySelector() +class TestStrategySelector(base.TestCase): - @patch.object(DefaultStrategyLoader, 'load') - def test_define_from_goal(self, mock_call): - cfg.CONF.set_override('goals', - {"DUMMY": "fake"}, group='watcher_goals', - enforce_type=True) + def setUp(self): + super(TestStrategySelector, self).setUp() + + @mock.patch.object(default_loader.DefaultStrategyLoader, 'load') + def test_select_with_strategy_name(self, m_load): expected_goal = 'DUMMY' - expected_strategy = CONF.watcher_goals.goals[expected_goal] - self.strategy_selector.define_from_goal(expected_goal, osc=None) - mock_call.assert_called_once_with(expected_strategy, osc=None) + expected_strategy = "dummy" + strategy_selector = default_selector.DefaultStrategySelector( + expected_goal, expected_strategy, osc=None) + strategy_selector.select() + m_load.assert_called_once_with(expected_strategy, osc=None) - @patch.object(DefaultStrategyLoader, 'load') - def test_define_from_goal_with_incorrect_mapping(self, mock_call): - cfg.CONF.set_override('goals', {}, group='watcher_goals', - enforce_type=True) - self.assertRaises(WatcherException, - self.strategy_selector.define_from_goal, - "DUMMY") - self.assertEqual(0, mock_call.call_count) + @mock.patch.object(default_loader.DefaultStrategyLoader, 'load') + @mock.patch.object(default_loader.DefaultStrategyLoader, 'list_available') + def test_select_with_goal_name_only(self, m_list_available, m_load): + m_list_available.return_value = {"dummy": strategies.DummyStrategy} + expected_goal = 'DUMMY' + expected_strategy = "dummy" + strategy_selector = default_selector.DefaultStrategySelector( + expected_goal, osc=None) + strategy_selector.select() + m_load.assert_called_once_with(expected_strategy, osc=None) + + def test_select_non_existing_strategy(self): + strategy_selector = default_selector.DefaultStrategySelector( + "DUMMY", "NOT_FOUND") + self.assertRaises(exception.LoadingError, strategy_selector.select) + + @mock.patch.object(default_loader.DefaultStrategyLoader, 'list_available') + def test_select_no_available_strategy_for_goal(self, m_list_available): + m_list_available.return_value = {} + strategy_selector = default_selector.DefaultStrategySelector( + "DUMMY") + self.assertRaises(exception.NoAvailableStrategyForGoal, + strategy_selector.select) diff --git a/watcher_tempest_plugin/tests/api/admin/test_audit_template.py b/watcher_tempest_plugin/tests/api/admin/test_audit_template.py index 19727b095..2bddbc644 100644 --- a/watcher_tempest_plugin/tests/api/admin/test_audit_template.py +++ b/watcher_tempest_plugin/tests/api/admin/test_audit_template.py @@ -137,7 +137,7 @@ class TestAuditTemplate(base.BaseInfraOptimTest): new_name = 'my at new name %s' % uuid.uuid4() new_description = 'my new at description' new_host_aggregate = 10 - new_goal = 'BASIC_CONSOLIDATION' + new_goal = 'SERVER_CONSOLIDATION' new_extra = {'key1': 'new-value1', 'key2': 'new-value2'} patch = [{'path': '/name', diff --git a/watcher_tempest_plugin/tests/scenario/test_execute_basic_optim.py b/watcher_tempest_plugin/tests/scenario/test_execute_basic_optim.py index 1f20a864d..60f2caa83 100644 --- a/watcher_tempest_plugin/tests/scenario/test_execute_basic_optim.py +++ b/watcher_tempest_plugin/tests/scenario/test_execute_basic_optim.py @@ -30,7 +30,7 @@ CONF = config.CONF class TestExecuteBasicStrategy(base.BaseInfraOptimScenarioTest): """Tests for action plans""" - BASIC_GOAL = "BASIC_CONSOLIDATION" + BASIC_GOAL = "SERVER_CONSOLIDATION" @classmethod def skip_checks(cls):