From 0b29a8394ba30800db8d77f5b185cd9b7bd366ad Mon Sep 17 00:00:00 2001 From: Edwin Zhai Date: Thu, 17 Mar 2016 01:50:39 +0000 Subject: [PATCH] Enable strategy parameters Strategy provides parameters to customize algorithm behavior. End user could query then specify parameters for their requirements. Change-Id: Id097db5f6e79c94b57674c8e5d55b06098abf18c Implements-bp: optimization-threshold --- watcher/api/controllers/v1/audit.py | 29 ++++++++++- watcher/api/controllers/v1/strategy.py | 6 +++ watcher/common/utils.py | 23 ++++++++ watcher/db/sqlalchemy/models.py | 2 + .../strategy/context/default.py | 11 ++++ .../strategy/strategies/base.py | 19 +++++++ .../strategy/strategies/dummy_strategy.py | 25 ++++++++- .../strategies/outlet_temp_control.py | 24 ++++++--- watcher/decision_engine/sync.py | 11 ++-- watcher/objects/audit.py | 1 + watcher/objects/strategy.py | 1 + watcher/tests/api/v1/test_audits.py | 52 +++++++++++++++++++ watcher/tests/db/utils.py | 2 + .../strategies/test_dummy_strategy.py | 5 ++ .../strategies/test_outlet_temp_control.py | 5 ++ watcher/tests/decision_engine/test_sync.py | 6 ++- 16 files changed, 209 insertions(+), 13 deletions(-) diff --git a/watcher/api/controllers/v1/audit.py b/watcher/api/controllers/v1/audit.py index 0e9d26725..aa258c390 100644 --- a/watcher/api/controllers/v1/audit.py +++ b/watcher/api/controllers/v1/audit.py @@ -60,6 +60,9 @@ class AuditPostType(wtypes.Base): state = wsme.wsattr(wtypes.text, readonly=True, default=objects.audit.State.PENDING) + parameters = wtypes.wsattr({wtypes.text: types.jsontype}, mandatory=False, + default={}) + def as_audit(self): audit_type_values = [val.value for val in objects.audit.AuditType] if self.type not in audit_type_values: @@ -68,7 +71,9 @@ class AuditPostType(wtypes.Base): return Audit( audit_template_id=self.audit_template_uuid, type=self.type, - deadline=self.deadline) + deadline=self.deadline, + parameters=self.parameters, + ) class AuditPatchType(types.JsonPatchType): @@ -148,6 +153,9 @@ class Audit(base.APIBase): mandatory=False) """The name of the audit template this audit refers to""" + parameters = {wtypes.text: types.jsontype} + """The strategy parameters for this audit""" + links = wsme.wsattr([link.Link], readonly=True) """A list containing a self link and associated audit links""" @@ -364,6 +372,25 @@ class AuditsController(rest.RestController): message=_('The audit template UUID or name specified is ' 'invalid')) + audit_template = objects.AuditTemplate.get(pecan.request.context, + audit._audit_template_uuid) + strategy_id = audit_template.strategy_id + no_schema = True + if strategy_id is not None: + # validate parameter when predefined strategy in audit template + strategy = objects.Strategy.get(pecan.request.context, strategy_id) + schema = strategy.parameters_spec + if schema: + # validate input parameter with default value feedback + no_schema = False + utils.DefaultValidatingDraft4Validator(schema).validate( + audit.parameters) + + if no_schema and audit.parameters: + raise exception.Invalid(_('Specify parameters but no predefined ' + 'strategy for audit template, or no ' + 'parameter spec in predefined strategy')) + audit_dict = audit.as_dict() context = pecan.request.context new_audit = objects.Audit(context, **audit_dict) diff --git a/watcher/api/controllers/v1/strategy.py b/watcher/api/controllers/v1/strategy.py index b23fbaa41..53a2cc335 100644 --- a/watcher/api/controllers/v1/strategy.py +++ b/watcher/api/controllers/v1/strategy.py @@ -112,6 +112,9 @@ class Strategy(base.APIBase): mandatory=False) """The name of the goal this audit refers to""" + parameters_spec = {wtypes.text: types.jsontype} + """ Parameters spec dict""" + def __init__(self, **kwargs): super(Strategy, self).__init__() @@ -121,11 +124,14 @@ class Strategy(base.APIBase): self.fields.append('display_name') self.fields.append('goal_uuid') self.fields.append('goal_name') + self.fields.append('parameters_spec') setattr(self, 'uuid', kwargs.get('uuid', wtypes.Unset)) setattr(self, 'name', kwargs.get('name', wtypes.Unset)) setattr(self, 'display_name', kwargs.get('display_name', wtypes.Unset)) setattr(self, 'goal_uuid', kwargs.get('goal_id', wtypes.Unset)) setattr(self, 'goal_name', kwargs.get('goal_id', wtypes.Unset)) + setattr(self, 'parameters_spec', kwargs.get('parameters_spec', + wtypes.Unset)) @staticmethod def _convert_with_links(strategy, url, expand=True): diff --git a/watcher/common/utils.py b/watcher/common/utils.py index c77b4302e..04b487155 100644 --- a/watcher/common/utils.py +++ b/watcher/common/utils.py @@ -16,6 +16,7 @@ """Utilities and helper functions.""" +from jsonschema import validators from oslo_config import cfg from oslo_log import log as logging @@ -129,3 +130,25 @@ def get_cls_import_path(cls): if module is None or module == str.__module__: return cls.__name__ return module + '.' + cls.__name__ + + +# Default value feedback extension as jsonschema doesn't support it +def extend_with_default(validator_class): + validate_properties = validator_class.VALIDATORS["properties"] + + def set_defaults(validator, properties, instance, schema): + for prop, subschema in properties.items(): + if "default" in subschema: + instance.setdefault(prop, subschema["default"]) + + for error in validate_properties( + validator, properties, instance, schema, + ): + yield error + + return validators.extend( + validator_class, {"properties": set_defaults}, + ) + +DefaultValidatingDraft4Validator = extend_with_default( + validators.Draft4Validator) diff --git a/watcher/db/sqlalchemy/models.py b/watcher/db/sqlalchemy/models.py index 245942dda..f2b38c25d 100644 --- a/watcher/db/sqlalchemy/models.py +++ b/watcher/db/sqlalchemy/models.py @@ -124,6 +124,7 @@ class Strategy(Base): name = Column(String(63), nullable=False) display_name = Column(String(63), nullable=False) goal_id = Column(Integer, ForeignKey('goals.id'), nullable=False) + parameters_spec = Column(JSONEncodedDict, nullable=True) class Goal(Base): @@ -175,6 +176,7 @@ class Audit(Base): deadline = Column(DateTime, nullable=True) audit_template_id = Column(Integer, ForeignKey('audit_templates.id'), nullable=False) + parameters = Column(JSONEncodedDict, nullable=True) class Action(Base): diff --git a/watcher/decision_engine/strategy/context/default.py b/watcher/decision_engine/strategy/context/default.py index 2a8f5c526..851c8b485 100644 --- a/watcher/decision_engine/strategy/context/default.py +++ b/watcher/decision_engine/strategy/context/default.py @@ -16,6 +16,7 @@ from oslo_log import log from watcher.common import clients +from watcher.common import utils from watcher.decision_engine.strategy.context import base from watcher.decision_engine.strategy.selection import default @@ -58,4 +59,14 @@ class DefaultStrategyContext(base.BaseStrategyContext): selected_strategy = strategy_selector.select() + schema = selected_strategy.get_schema() + if not audit.parameters and schema: + # Default value feedback if no predefined strategy + utils.DefaultValidatingDraft4Validator(schema).validate( + audit.parameters) + + selected_strategy.input_parameters.update({ + name: value for name, value in audit.parameters.items() + }) + return selected_strategy.execute() diff --git a/watcher/decision_engine/strategy/strategies/base.py b/watcher/decision_engine/strategy/strategies/base.py index 86c8bc7c2..0c3124f70 100644 --- a/watcher/decision_engine/strategy/strategies/base.py +++ b/watcher/decision_engine/strategy/strategies/base.py @@ -41,6 +41,7 @@ import six from watcher.common import clients from watcher.common.loader import loadable +from watcher.common import utils from watcher.decision_engine.loading import default as loading from watcher.decision_engine.solution import default from watcher.decision_engine.strategy.common import level @@ -76,6 +77,7 @@ class BaseStrategy(loadable.Loadable): self._collector_manager = None self._model = None self._goal = None + self._input_parameters = utils.Struct() @classmethod @abc.abstractmethod @@ -176,6 +178,23 @@ class BaseStrategy(loadable.Loadable): return self._model + @classmethod + def get_schema(cls): + """Defines a Schema that the input parameters shall comply to + + :return: A jsonschema format (mandatory default setting) + :rtype: dict + """ + return {} + + @property + def input_parameters(self): + return self._input_parameters + + @input_parameters.setter + def input_parameters(self, p): + self._input_parameters = p + @property def osc(self): if not self._osc: diff --git a/watcher/decision_engine/strategy/strategies/dummy_strategy.py b/watcher/decision_engine/strategy/strategies/dummy_strategy.py index 63d265e57..253b6e6ff 100644 --- a/watcher/decision_engine/strategy/strategies/dummy_strategy.py +++ b/watcher/decision_engine/strategy/strategies/dummy_strategy.py @@ -54,7 +54,10 @@ class DummyStrategy(base.DummyBaseStrategy): raise exception.ClusterStateNotDefined() def do_execute(self): - LOG.debug("Executing Dummy strategy") + para1 = self.input_parameters.para1 + para2 = self.input_parameters.para2 + LOG.debug("Executing Dummy strategy with para1=%(p1)f, para2=%(p2)s", + {'p1': para1, 'p2': para2}) parameters = {'message': 'hello World'} self.solution.add_action(action_type=self.NOP, input_parameters=parameters) @@ -80,3 +83,23 @@ class DummyStrategy(base.DummyBaseStrategy): @classmethod def get_translatable_display_name(cls): return "Dummy strategy" + + @classmethod + def get_schema(cls): + # Mandatory default setting for each element + return { + "properties": { + "para1": { + "description": "number parameter example", + "type": "number", + "default": 3.2, + "minimum": 1.0, + "maximum": 10.2, + }, + "para2": { + "description": "string parameter example", + "type": "string", + "default": "hello" + }, + }, + } diff --git a/watcher/decision_engine/strategy/strategies/outlet_temp_control.py b/watcher/decision_engine/strategy/strategies/outlet_temp_control.py index 0a72c9682..dbe9f714b 100644 --- a/watcher/decision_engine/strategy/strategies/outlet_temp_control.py +++ b/watcher/decision_engine/strategy/strategies/outlet_temp_control.py @@ -73,8 +73,6 @@ class OutletTempControl(base.ThermalOptimizationBaseStrategy): # The meter to report outlet temperature in ceilometer METER_NAME = "hardware.ipmi.node.outlet_temperature" - # Unit: degree C - THRESHOLD = 35.0 MIGRATION = "migrate" @@ -87,10 +85,6 @@ class OutletTempControl(base.ThermalOptimizationBaseStrategy): :type osc: :py:class:`~.OpenStackClients` instance, optional """ super(OutletTempControl, self).__init__(config, osc) - # the migration plan will be triggered when the outlet temperature - # reaches threshold - # TODO(zhenzanz): Threshold should be configurable for each audit - self.threshold = self.THRESHOLD self._meter = self.METER_NAME self._ceilometer = None @@ -106,6 +100,19 @@ class OutletTempControl(base.ThermalOptimizationBaseStrategy): def get_translatable_display_name(cls): return "Outlet temperature based strategy" + @classmethod + def get_schema(cls): + # Mandatory default setting for each element + return { + "properties": { + "threshold": { + "description": "temperature threshold for migration", + "type": "number", + "default": 35.0 + }, + }, + } + @property def ceilometer(self): if self._ceilometer is None: @@ -224,6 +231,11 @@ class OutletTempControl(base.ThermalOptimizationBaseStrategy): raise wexc.ClusterStateNotDefined() def do_execute(self): + # the migration plan will be triggered when the outlet temperature + # reaches threshold + self.threshold = self.input_parameters.threshold + LOG.debug("Initializing Outlet temperature strategy with threshold=%d", + self.threshold) hosts_need_release, hosts_target = self.group_hosts_by_outlet_temp() if len(hosts_need_release) == 0: diff --git a/watcher/decision_engine/sync.py b/watcher/decision_engine/sync.py index cd7676ec5..212f06c1f 100644 --- a/watcher/decision_engine/sync.py +++ b/watcher/decision_engine/sync.py @@ -28,7 +28,8 @@ LOG = log.getLogger(__name__) GoalMapping = collections.namedtuple( 'GoalMapping', ['name', 'display_name', 'efficacy_specification']) StrategyMapping = collections.namedtuple( - 'StrategyMapping', ['name', 'goal_name', 'display_name']) + 'StrategyMapping', + ['name', 'goal_name', 'display_name', 'parameters_spec']) IndicatorSpec = collections.namedtuple( 'IndicatorSpec', ['name', 'description', 'unit', 'schema']) @@ -90,7 +91,8 @@ class Syncer(object): self._available_strategies_map = { StrategyMapping( name=s.name, goal_name=goals_map[s.goal_id], - display_name=s.display_name): s + display_name=s.display_name, + parameters_spec=str(s.parameters_spec)): s for s in self.available_strategies } return self._available_strategies_map @@ -148,6 +150,7 @@ class Syncer(object): strategy_name = strategy_map.name strategy_display_name = strategy_map.display_name goal_name = strategy_map.goal_name + parameters_spec = strategy_map.parameters_spec strategy_mapping = dict() # Strategies that are matching by name with the given @@ -162,6 +165,7 @@ class Syncer(object): strategy.name = strategy_name strategy.display_name = strategy_display_name strategy.goal_id = objects.Goal.get_by_name(self.ctx, goal_name).id + strategy.parameters_spec = parameters_spec strategy.create() LOG.info(_LI("Strategy %s created"), strategy_name) @@ -284,7 +288,8 @@ class Syncer(object): strategies_map[strategy_cls.get_name()] = StrategyMapping( name=strategy_cls.get_name(), goal_name=strategy_cls.get_goal_name(), - display_name=strategy_cls.get_translatable_display_name()) + display_name=strategy_cls.get_translatable_display_name(), + parameters_spec=str(strategy_cls.get_schema())) return discovered_map diff --git a/watcher/objects/audit.py b/watcher/objects/audit.py index df65ebffb..ac236ead6 100644 --- a/watcher/objects/audit.py +++ b/watcher/objects/audit.py @@ -85,6 +85,7 @@ class Audit(base.WatcherObject): 'state': obj_utils.str_or_none, 'deadline': obj_utils.datetime_or_str_or_none, 'audit_template_id': obj_utils.int_or_none, + 'parameters': obj_utils.dict_or_none, } @staticmethod diff --git a/watcher/objects/strategy.py b/watcher/objects/strategy.py index 85005434d..60b35701c 100644 --- a/watcher/objects/strategy.py +++ b/watcher/objects/strategy.py @@ -31,6 +31,7 @@ class Strategy(base.WatcherObject): 'name': obj_utils.str_or_none, 'display_name': obj_utils.str_or_none, 'goal_id': obj_utils.int_or_none, + 'parameters_spec': obj_utils.dict_or_none, } @staticmethod diff --git a/watcher/tests/api/v1/test_audits.py b/watcher/tests/api/v1/test_audits.py index 7ffc2878d..d661aa8cb 100644 --- a/watcher/tests/api/v1/test_audits.py +++ b/watcher/tests/api/v1/test_audits.py @@ -39,6 +39,21 @@ def post_get_test_audit(**kw): return audit +def post_get_test_audit_with_predefined_strategy(**kw): + spec = kw.pop('strategy_parameters_spec', {}) + strategy_id = 2 + strategy = db_utils.get_test_strategy(parameters_spec=spec, id=strategy_id) + + audit_template = db_utils.get_test_audit_template( + strategy_id=strategy['id']) + + audit = api_utils.audit_post_data(**kw) + del audit['audit_template_id'] + audit['audit_template_uuid'] = audit_template['uuid'] + + return audit + + class TestAuditObject(base.TestCase): def test_audit_init(self): @@ -563,6 +578,43 @@ class TestPost(api_base.FunctionalTest): self.assertEqual(400, response.status_int) assert not mock_trigger_audit.called + @mock.patch.object(deapi.DecisionEngineAPI, 'trigger_audit') + def test_create_audit_parameters_no_predefined_strategy( + self, mock_trigger_audit): + mock_trigger_audit.return_value = mock.ANY + audit_dict = post_get_test_audit(parameters={'name': 'Tom'}) + del audit_dict['uuid'] + del audit_dict['state'] + + response = self.post_json('/audits', audit_dict, expect_errors=True) + self.assertEqual('application/json', response.content_type) + self.assertEqual(400, response.status_int) + expected_error_msg = ('Specify parameters but no predefined ' + 'strategy for audit template, or no ' + 'parameter spec in predefined strategy') + self.assertTrue(response.json['error_message']) + self.assertTrue(expected_error_msg in response.json['error_message']) + assert not mock_trigger_audit.called + + @mock.patch.object(deapi.DecisionEngineAPI, 'trigger_audit') + def test_create_audit_parameters_no_schema( + self, mock_trigger_audit): + mock_trigger_audit.return_value = mock.ANY + audit_dict = post_get_test_audit_with_predefined_strategy( + parameters={'name': 'Tom'}) + del audit_dict['uuid'] + del audit_dict['state'] + + response = self.post_json('/audits', audit_dict, expect_errors=True) + self.assertEqual('application/json', response.content_type) + self.assertEqual(400, response.status_int) + expected_error_msg = ('Specify parameters but no predefined ' + 'strategy for audit template, or no ' + 'parameter spec in predefined strategy') + self.assertTrue(response.json['error_message']) + self.assertTrue(expected_error_msg in response.json['error_message']) + assert not mock_trigger_audit.called + # class TestDelete(api_base.FunctionalTest): diff --git a/watcher/tests/db/utils.py b/watcher/tests/db/utils.py index 55702636a..793914d03 100644 --- a/watcher/tests/db/utils.py +++ b/watcher/tests/db/utils.py @@ -61,6 +61,7 @@ def get_test_audit(**kwargs): 'created_at': kwargs.get('created_at'), 'updated_at': kwargs.get('updated_at'), 'deleted_at': kwargs.get('deleted_at'), + 'parameters': kwargs.get('parameters', {}), } @@ -178,6 +179,7 @@ def get_test_strategy(**kwargs): 'created_at': kwargs.get('created_at'), 'updated_at': kwargs.get('updated_at'), 'deleted_at': kwargs.get('deleted_at'), + 'parameters_spec': kwargs.get('parameters_spec', {}) } diff --git a/watcher/tests/decision_engine/strategy/strategies/test_dummy_strategy.py b/watcher/tests/decision_engine/strategy/strategies/test_dummy_strategy.py index 72a1d9a98..155716c17 100644 --- a/watcher/tests/decision_engine/strategy/strategies/test_dummy_strategy.py +++ b/watcher/tests/decision_engine/strategy/strategies/test_dummy_strategy.py @@ -17,6 +17,7 @@ import mock from watcher.applier.loading import default +from watcher.common import utils from watcher.decision_engine.model import model_root from watcher.decision_engine.strategy import strategies from watcher.tests import base @@ -45,12 +46,16 @@ class TestDummyStrategy(base.TestCase): def test_dummy_strategy(self): dummy = strategies.DummyStrategy(config=mock.Mock()) + dummy.input_parameters = utils.Struct() + dummy.input_parameters.update({'para1': 4.0, 'para2': 'Hi'}) solution = dummy.execute() self.assertEqual(3, len(solution.actions)) def test_check_parameters(self): model = self.fake_cluster.generate_scenario_3_with_2_hypervisors() self.m_model.return_value = model + self.strategy.input_parameters = utils.Struct() + self.strategy.input_parameters.update({'para1': 4.0, 'para2': 'Hi'}) solution = self.strategy.execute() loader = default.DefaultActionLoader() for action in solution.actions: diff --git a/watcher/tests/decision_engine/strategy/strategies/test_outlet_temp_control.py b/watcher/tests/decision_engine/strategy/strategies/test_outlet_temp_control.py index 82e9c1195..fa62c7e1d 100644 --- a/watcher/tests/decision_engine/strategy/strategies/test_outlet_temp_control.py +++ b/watcher/tests/decision_engine/strategy/strategies/test_outlet_temp_control.py @@ -21,6 +21,7 @@ import mock from watcher.applier.loading import default from watcher.common import exception +from watcher.common import utils from watcher.decision_engine.model import model_root from watcher.decision_engine.model import resource from watcher.decision_engine.strategy import strategies @@ -57,6 +58,10 @@ class TestOutletTempControl(base.BaseTestCase): statistic_aggregation=self.fake_metrics.mock_get_statistics) self.strategy = strategies.OutletTempControl(config=mock.Mock()) + self.strategy.input_parameters = utils.Struct() + self.strategy.input_parameters.update({'threshold': 34.3}) + self.strategy.threshold = 34.3 + def test_calc_used_res(self): model = self.fake_cluster.generate_scenario_3_with_2_hypervisors() self.m_model.return_value = model diff --git a/watcher/tests/decision_engine/test_sync.py b/watcher/tests/decision_engine/test_sync.py index 7c588e3df..95b97e913 100644 --- a/watcher/tests/decision_engine/test_sync.py +++ b/watcher/tests/decision_engine/test_sync.py @@ -154,7 +154,8 @@ class TestSyncer(base.DbTestCase): ] m_s_list.return_value = [ objects.Strategy(self.ctx, id=1, name="strategy_1", - goal_id=1, display_name="Strategy 1") + goal_id=1, display_name="Strategy 1", + parameters_spec='{}') ] self.syncer.sync() @@ -218,7 +219,8 @@ class TestSyncer(base.DbTestCase): ] m_s_list.return_value = [ objects.Strategy(self.ctx, id=1, name="strategy_1", - goal_id=1, display_name="original") + goal_id=1, display_name="original", + parameters_spec='{}') ] self.syncer.sync()