Fix div-by-zero when svc weight is 0 for all masters in location

The relative service weight output tries to show the relative
chance that any given master in a locaiton will be picked. This
didn't account for all masters having a weight of 0 which would
result in a divide-by-zero error.

Implement the following rules:
1. If all masters have weight == 0 then all are equally
   weighted.
2. If any masters have weight == 0 then they have an
   extremely small chance of being chosen, percentage is
   0.1.
3. Otherwise it's percentage change is based on the sum of
   the weights of non-zero masters.

https://pagure.io/freeipa/issue/8135

Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>

ipaserver/plugins/location.py

@@ -224,10 +224,30 @@ class location_show(LDAPRetrieve):
                 if 'DNS server' in s_roles:
                     dns_servers.append(s_name)
 
+            # 1. If all masters have weight == 0 then all are equally
+            #    weighted.
+            # 2. If any masters have weight == 0 then they have an
+            #    extremely small chance of being chosen, percentage is
+            #    0.1.
+            # 3. Otherwise it's percentage change is based on the sum of
+            #    the weights of non-zero masters.
+            dividend = 100.0
+            if weight_sum != 0:
+                for server in servers_additional_info.values():
+                    if int(server['ipaserviceweight'][0]) == 0:
+                        dividend = dividend - 0.1
             for server in servers_additional_info.values():
+                if weight_sum == 0:
+                    val = 100 / len(servers)
+                elif int(server['ipaserviceweight'][0]) == 0:
+                    val = 0.1
+                else:
+                    val = (
+                        int(server['ipaserviceweight'][0]) *
+                        dividend / weight_sum
+                    )
                 server['service_relative_weight'] = [
-                    u'{:.1f}%'.format(
+                    u'{:.1f}%'.format(val)
-                        int(server['ipaserviceweight'][0])*100.0/weight_sum)
                 ]
             if servers_name:
                 result['result']['servers_server'] = servers_name

ipatests/test_integration/test_dns_locations.py

@@ -2,6 +2,7 @@
 # Copyright (C) 2016  FreeIPA Contributors see COPYING for license
 #
 import logging
+import re
 import time
 import pytest
 import dns.resolver
@@ -82,6 +83,11 @@ def _gen_expected_a_rrset(rname, servers, ttl=86400):
                                     dns.rdatatype.A, servers)
 
 
+def _get_relative_weights(text):
+    """Takes location-show output and returns a list of percentages"""
+    return re.findall(r"\d+.\d%", text)
+
+
 class TestDNSLocations(IntegrationTest):
     """Simple test if SRV DNS records for IPA locations are generated properly
 
@@ -346,6 +352,81 @@ class TestDNSLocations(IntegrationTest):
             self._test_SRV_rec_against_server(ip, domain_paris_loc,
                                               servers_paris_loc)
 
+    def test_change_weight_relative_zero_0(self):
+        """Change weight of one master and check on relative weight %
+        """
+
+        new_weight = 0
+
+        # Put all servers into one location
+        self.master.run_command([
+            'ipa', 'server-mod', self.replicas[0].hostname, '--location',
+            self.LOC_PARIS])
+
+        # Modify one to have a weight of 0
+        result = self.master.run_command([
+            'ipa', 'server-mod', self.master.hostname, '--service-weight',
+            str(new_weight)
+        ])
+
+        result = self.master.run_command([
+            'ipa', 'location-show', self.LOC_PARIS
+        ])
+        weights = _get_relative_weights(result.stdout_text)
+        assert weights.count('0.1%') == 1
+        assert weights.count('50.0%') == 2
+
+    # The following three tests are name-sensitive so they run in
+    # a specific order. They use the paris location and depend on
+    # the existing values of the server location and weight to work
+    # properly
+    def test_change_weight_relative_zero_1(self):
+        """Change all weights to zero and ensure no div by zero
+        """
+
+        new_weight = 0
+
+        # Depends on order of test execution but all masters are now
+        # in LOC_PARIS and self.master has a weight of 0.
+
+        # Modify all replicas to have a weight of 0
+        for hostname in (self.replicas[0].hostname, self.replicas[1].hostname):
+            self.master.run_command([
+                'ipa', 'server-mod', hostname, '--service-weight',
+                str(new_weight)
+            ])
+
+        result = self.master.run_command([
+            'ipa', 'location-show', self.LOC_PARIS
+        ])
+        weights = _get_relative_weights(result.stdout_text)
+        assert weights.count('33.3%') == 3
+
+    def test_change_weight_relative_zero_2(self):
+        """Change to mixed weight values and check percentages
+        """
+
+        new_weight = 100
+
+        # Change master to be primary, replicas secondary
+        self.master.run_command([
+            'ipa', 'server-mod', self.master.hostname, '--service-weight',
+            '200'
+        ])
+        for hostname in (self.replicas[0].hostname,
+                         self.replicas[1].hostname):
+            self.master.run_command([
+                'ipa', 'server-mod', hostname, '--service-weight',
+                str(new_weight)
+            ])
+
+        result = self.master.run_command([
+            'ipa', 'location-show', self.LOC_PARIS
+        ])
+        weights = _get_relative_weights(result.stdout_text)
+        assert weights.count('50.0%') == 1
+        assert weights.count('25.0%') == 2
+
     def test_restore_locations_and_weight(self):
         """Restore locations and weight. Not just for test purposes but also
         for the following tests"""