From dd119ca1f808f492a69e7bf45d3dd525b1a08c6e Mon Sep 17 00:00:00 2001 From: Dantali0n Date: Tue, 11 Jun 2019 11:31:37 +0200 Subject: [PATCH] Backwards compatibility for node parameter Adds backwards compatibility for node parameter used by strategies. If the node value is set by the user configuration it will override the value for compute_node which is the value used by the strategies now. This change was introduced in: https://review.opendev.org/#/c/656622/ Resolution discussed in the meeting on the 5th of June 2019 https://eavesdrop.openstack.org/meetings/watcher/2019/watcher.2019-06-05-08.00.log.html Change-Id: Idaea062789a6b169e64f556fecc34cfbaaee5076 --- ...rface-implementation-222769d55a127d33.yaml | 17 +++++++ .../strategies/basic_consolidation.py | 27 +++++++++-- .../strategies/workload_stabilization.py | 38 ++++++++++++++- .../strategies/test_basic_consolidation.py | 15 ++++++ .../strategies/test_workload_stabilization.py | 47 ++++++++++++++++--- 5 files changed, 133 insertions(+), 11 deletions(-) create mode 100644 releasenotes/notes/formal-datasource-interface-implementation-222769d55a127d33.yaml diff --git a/releasenotes/notes/formal-datasource-interface-implementation-222769d55a127d33.yaml b/releasenotes/notes/formal-datasource-interface-implementation-222769d55a127d33.yaml new file mode 100644 index 000000000..c5164b7fd --- /dev/null +++ b/releasenotes/notes/formal-datasource-interface-implementation-222769d55a127d33.yaml @@ -0,0 +1,17 @@ +--- +features: + - | + Improved interface for datasource baseclass that better defines expected + values and types for parameters and return types of all abstract methods. + This allows all strategies to work with every datasource provided the + metrics are configured for that given datasource. +deprecations: + - | + The new strategy baseclass has significant changes in method parameters + and any out-of-tree strategies will have to be adopted. + - | + Several strategies have changed the `node` parameter to `compute_node` to + be better aligned with terminology. These strategies include + `basic_consolidation` and `workload_stabilzation`. The `node` parameter + will remain supported during Train release and will be removed in the + subsequent release. diff --git a/watcher/decision_engine/strategy/strategies/basic_consolidation.py b/watcher/decision_engine/strategy/strategies/basic_consolidation.py index 7db961a8f..82e042b0b 100644 --- a/watcher/decision_engine/strategy/strategies/basic_consolidation.py +++ b/watcher/decision_engine/strategy/strategies/basic_consolidation.py @@ -95,8 +95,12 @@ class BasicConsolidation(base.ServerConsolidationBaseStrategy): @property def aggregation_method(self): return self.input_parameters.get( - 'aggregation_method', - {"instance": 'mean', "compute_node": 'mean'}) + 'aggregation_method', { + "instance": 'mean', + "compute_node": 'mean', + "node": '' + } + ) @classmethod def get_display_name(cls): @@ -148,8 +152,18 @@ class BasicConsolidation(base.ServerConsolidationBaseStrategy): "type": "string", "default": 'mean' }, + "node": { + "type": "string", + # node is deprecated + "default": '' + }, }, - "default": {"instance": 'mean', "compute_node": 'mean'} + "default": { + "instance": 'mean', + "compute_node": 'mean', + # node is deprecated + "node": '', + } }, }, } @@ -391,6 +405,13 @@ class BasicConsolidation(base.ServerConsolidationBaseStrategy): def pre_execute(self): self._pre_execute() + # backwards compatibility for node parameter. + if self.aggregation_method['node'] is not '': + LOG.warning('Parameter node has been renamed to compute_node and ' + 'will be removed in next release.') + self.aggregation_method['compute_node'] = \ + self.aggregation_method['node'] + def do_execute(self): unsuccessful_migration = 0 diff --git a/watcher/decision_engine/strategy/strategies/workload_stabilization.py b/watcher/decision_engine/strategy/strategies/workload_stabilization.py index ece3686d1..1904fad7a 100644 --- a/watcher/decision_engine/strategy/strategies/workload_stabilization.py +++ b/watcher/decision_engine/strategy/strategies/workload_stabilization.py @@ -193,8 +193,19 @@ class WorkloadStabilization(base.WorkloadStabilizationBaseStrategy): "type": "integer", "minimum": 0 }, + "node": { + "type": "integer", + # node is deprecated + "minimum": 0, + "default": 0 + }, }, - "default": {"instance": 720, "compute_node": 600} + "default": { + "instance": 720, + "compute_node": 600, + # node is deprecated + "node": 0, + } }, "aggregation_method": { "description": "Function used to aggregate multiple " @@ -213,8 +224,18 @@ class WorkloadStabilization(base.WorkloadStabilizationBaseStrategy): "type": "string", "default": 'mean' }, + # node is deprecated + "node": { + "type": "string", + "default": '' + }, }, - "default": {"instance": 'mean', "compute_node": 'mean'} + "default": { + "instance": 'mean', + "compute_node": 'mean', + # node is deprecated + "node": '', + } }, "granularity": { "description": "The time between two measures in an " @@ -485,6 +506,19 @@ class WorkloadStabilization(base.WorkloadStabilizationBaseStrategy): self.periods = self.input_parameters.periods self.aggregation_method = self.input_parameters.aggregation_method + # backwards compatibility for node parameter with aggregate. + if self.aggregation_method['node'] is not '': + LOG.warning('Parameter node has been renamed to compute_node and ' + 'will be removed in next release.') + self.aggregation_method['compute_node'] = \ + self.aggregation_method['node'] + + # backwards compatibility for node parameter with period. + if self.periods['node'] is not 0: + LOG.warning('Parameter node has been renamed to compute_node and ' + 'will be removed in next release.') + self.periods['compute_node'] = self.periods['node'] + def do_execute(self): migration = self.check_threshold() if migration: diff --git a/watcher/tests/decision_engine/strategy/strategies/test_basic_consolidation.py b/watcher/tests/decision_engine/strategy/strategies/test_basic_consolidation.py index 0ce306966..82360e1fd 100644 --- a/watcher/tests/decision_engine/strategy/strategies/test_basic_consolidation.py +++ b/watcher/tests/decision_engine/strategy/strategies/test_basic_consolidation.py @@ -235,3 +235,18 @@ class TestBasicConsolidation(TestBaseStrategy): loaded_action = loader.load(action['action_type']) loaded_action.input_parameters = action['input_parameters'] loaded_action.validate_parameters() + + def test_parameter_backwards_compat(self): + # Set the deprecated node values to a none default value + self.strategy.input_parameters.update( + {'aggregation_method': { + "instance": "mean", + "compute_node": "mean", + "node": 'min'}}) + + # Pre execute method handles backwards compatibility of parameters + self.strategy.pre_execute() + + # assert that the compute_node values are updated to the those of node + self.assertEqual( + 'min', self.strategy.aggregation_method['compute_node']) diff --git a/watcher/tests/decision_engine/strategy/strategies/test_workload_stabilization.py b/watcher/tests/decision_engine/strategy/strategies/test_workload_stabilization.py index c576629fe..167b0e7d4 100644 --- a/watcher/tests/decision_engine/strategy/strategies/test_workload_stabilization.py +++ b/watcher/tests/decision_engine/strategy/strategies/test_workload_stabilization.py @@ -85,9 +85,14 @@ class TestWorkloadStabilization(TestBaseStrategy): "instance_ram_usage": "host_ram_usage"}, 'host_choice': 'retry', 'retry_count': 1, - 'periods': {"instance": 720, "compute_node": 600}, - 'aggregation_method': {"instance": "mean", - "compute_node": "mean"}}) + 'periods': { + "instance": 720, + "compute_node": 600, + "node": 0}, + 'aggregation_method': { + "instance": "mean", + "compute_node": "mean", + "node": ''}}) self.strategy.metrics = ["instance_cpu_usage", "instance_ram_usage"] self.strategy.thresholds = {"instance_cpu_usage": 0.2, "instance_ram_usage": 0.2} @@ -98,9 +103,18 @@ class TestWorkloadStabilization(TestBaseStrategy): "instance_ram_usage": "host_ram_usage"} self.strategy.host_choice = 'retry' self.strategy.retry_count = 1 - self.strategy.periods = {"instance": 720, "compute_node": 600} - self.strategy.aggregation_method = {"instance": "mean", - "compute_node": "mean"} + self.strategy.periods = { + "instance": 720, + "compute_node": 600, + # node is deprecated + "node": 0, + } + self.strategy.aggregation_method = { + "instance": "mean", + "compute_node": "mean", + # node is deprecated + "node": '', + } def test_get_instance_load(self): model = self.fake_c_cluster.generate_scenario_1() @@ -246,3 +260,24 @@ class TestWorkloadStabilization(TestBaseStrategy): with mock.patch.object(self.strategy, 'migrate') as mock_migrate: self.strategy.execute() mock_migrate.assert_not_called() + + def test_parameter_backwards_compat(self): + # Set the deprecated node values to a none default value + self.strategy.input_parameters.update( + {'periods': { + "instance": 720, + "compute_node": 600, + "node": 500 + }, 'aggregation_method': { + "instance": "mean", + "compute_node": "mean", + "node": 'min'}}) + + # Pre execute method handles backwards compatibility of parameters + self.strategy.pre_execute() + + # assert that the compute_node values are updated to the those of node + self.assertEqual( + 'min', self.strategy.aggregation_method['compute_node']) + self.assertEqual( + 500, self.strategy.periods['compute_node'])