Don't recompute weighers' minval/maxval attributes

Changing the minval/maxval attribute to the minimum/maxium of every
weigher run changes the outcome of future runs. We noticed it in the
SoftAffinityWeigher, where a previous run with a host hosting a lot of
instances for a server-group would make a later run use that maximum.
This resulted in the weight being lower than 1 for a host hosting all
instances of another server-group, if the number of instances of that
server-group on that host is less than a previous server-group's
instances on any host.

Previously, there were two places that computed the maxval/minval - once
in normalize() and once in weigh_objects() - but only the one in
weigh_objects() saved the values to the weigher.

The code now uses the maxval/minval as defined by the weigher and keeps
the weights inside the maxval-minval range. There's also only one place
to compute the minval/maxval now, if the weigher did not set a value:
normalize().

Closes-Bug: 1870096

Change-Id: I60a90dabcd21b4e049e218c7c55fa075bb7ff933

nova/tests/unit/scheduler/weights/test_weights_affinity.py

@@ -29,17 +29,16 @@ class SoftWeigherTestBase(test.NoDBTestCase):
         self.weight_handler = weights.HostWeightHandler()
         self.weighers = []
 
-    def _get_weighed_host(self, hosts, policy):
+    def _get_weighed_host(self, hosts, policy, group='default'):
+        if group == 'default':
+            members = ['member1', 'member2', 'member3', 'member4', 'member5',
+                'member6', 'member7']
+        else:
+            members = ['othermember1', 'othermember2']
         request_spec = objects.RequestSpec(
             instance_group=objects.InstanceGroup(
                 policy=policy,
-                members=['member1',
-                         'member2',
-                         'member3',
-                         'member4',
-                         'member5',
-                         'member6',
-                         'member7']))
+                members=members))
         return self.weight_handler.get_weighed_objects(self.weighers,
                                                        hosts,
                                                        request_spec)[0]
@@ -55,6 +54,8 @@ class SoftWeigherTestBase(test.NoDBTestCase):
                 'member3': mock.sentinel,
                 'member4': mock.sentinel,
                 'member5': mock.sentinel,
+                'othermember1': mock.sentinel,
+                'othermember2': mock.sentinel,
                 'instance14': mock.sentinel
             }}),
             ('host3', 'node3', {'instances': {
@@ -68,11 +69,11 @@ class SoftWeigherTestBase(test.NoDBTestCase):
         return [fakes.FakeHostState(host, node, values)
                 for host, node, values in host_values]
 
-    def _do_test(self, policy, expected_weight,
-                 expected_host):
+    def _do_test(self, policy, expected_weight, expected_host,
+                 group='default'):
         hostinfo_list = self._get_all_hosts()
         weighed_host = self._get_weighed_host(hostinfo_list,
-                                              policy)
+                                              policy, group)
         self.assertEqual(expected_weight, weighed_host.weight)
         if expected_host:
             self.assertEqual(expected_host, weighed_host.obj.host)
@@ -159,6 +160,21 @@ class SoftAffinityWeigherTestCase(SoftWeigherTestBase):
         self.assertEqual(1.5, weighed_host.weight)
         self.assertEqual('host2', weighed_host.obj.host)
 
+    def test_running_twice(self):
+        """Run the weighing twice for different groups each run
+
+        The first run has a group with more members on the same host than the
+        second both. In both cases, most members of their groups are on the
+        same host => weight should be maximum (1 with default multiplier).
+        """
+        self._do_test(policy='soft-affinity',
+                      expected_weight=1.0,
+                      expected_host='host2')
+        self._do_test(policy='soft-affinity',
+                      expected_weight=1.0,
+                      expected_host='host2',
+                      group='other')
+
 
 class SoftAntiAffinityWeigherTestCase(SoftWeigherTestBase):
 

nova/weights.py

@@ -106,17 +106,11 @@ class BaseWeigher(object):
         for obj in weighed_obj_list:
             weight = self._weigh_object(obj.obj, weight_properties)
 
-            # Record the min and max values if they are None. If they are
-            # anything but none, we assume that the weigher had set them.
-            if self.minval is None:
-                self.minval = weight
-            if self.maxval is None:
-                self.maxval = weight
-
-            if weight < self.minval:
-                self.minval = weight
-            elif weight > self.maxval:
-                self.maxval = weight
+            # don't let the weight go beyond the defined max/min
+            if self.minval is not None:
+                weight = max(weight, self.minval)
+            if self.maxval is not None:
+                weight = min(weight, self.maxval)
 
             weights.append(weight)