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

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.

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',
+            'aggregation_method', {
-            {"instance": 'mean', "compute_node": 'mean'})
+                "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
 

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:

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'])

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},
+             'periods': {
-             'aggregation_method': {"instance": "mean",
+                 "instance": 720,
-                                    "compute_node": "mean"}})
+                 "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.periods = {
-        self.strategy.aggregation_method = {"instance": "mean",
+            "instance": 720,
-                                            "compute_node": "mean"}
+            "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'])