Merge lp:~blake-rouse/maas/reap-old-regiond-workers-2.1 into lp:maas/2.1

[Blake Rouse]

Backport r5728 + r5737: Reap dead sibling regiond workers. Import message of degraded rackd.

This fixes users seeing bad reports of bad connections when dead regiond workers are stuck in the database for a longer period of time. As long as one regiond process is running on that region controller it will ensure that its other siblings are not in the database unless they are actually running.

[Blake Rouse (blake-rouse)]

Self-approving backport.

review: Approve

=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py	2016-12-07 11:26:49 +0000
+++ src/maasserver/models/node.py	2017-02-16 16:52:53 +0000
@@ -4356,15 +4356,13 @@
                 Service.objects.update_service_for(
                     self, "rackd", SERVICE_STATUS.RUNNING)
             else:
-                # Not connected to all regions.
-                missing_regions = set(
-                    process.region
-                    for process in missing_processes
-                )
+                # Calculate precentage of connection.
+                percentage = ((dead_regions * 4) + len(missing_processes)) / (
+                    RegionController.objects.count() * 4)
                 Service.objects.update_service_for(
                     self, "rackd", SERVICE_STATUS.DEGRADED,
-                    "Missing connections to %d region controller(s)." % (
-                        dead_regions + len(missing_regions)))
+                    "{:.0%} connected to region controllers.".format(
+                        percentage))
 
     def get_image_sync_status(self, boot_images=None):
         """Return the status of the boot image import process."""

=== modified file 'src/maasserver/models/tests/test_node.py'
--- src/maasserver/models/tests/test_node.py	2016-12-07 11:26:49 +0000
+++ src/maasserver/models/tests/test_node.py	2017-02-16 16:52:53 +0000
@@ -8419,13 +8419,15 @@
         RegionRackRPCConnection.objects.create(
             endpoint=connected_endpoint, rack_controller=rack_controller)
         rack_controller.update_rackd_status()
+        percentage = (
+            (len(regions_without_processes) * 4) + 1) / (
+            (len(regions_without_processes) + len(regions_with_processes)) * 4)
         self.assertThat(
             Service.objects.get(node=rack_controller, name="rackd"),
             MatchesStructure.byEquality(
                 status=SERVICE_STATUS.DEGRADED, status_info=(
-                    "Missing connections to %d region controller(s)." % (
-                        len(regions_with_processes) +
-                        len(regions_without_processes)))))
+                    "{:.0%} connected to region controllers.".format(
+                        percentage))))
 
     fake_images = [
         {

=== modified file 'src/maasserver/rpc/regionservice.py'
--- src/maasserver/rpc/regionservice.py	2016-10-28 15:58:32 +0000
+++ src/maasserver/rpc/regionservice.py	2017-02-16 16:52:53 +0000
@@ -83,6 +83,7 @@
     get_all_interface_addresses,
     resolves_to_loopback_address,
 )
+from provisioningserver.utils.ps import is_pid_running
 from provisioningserver.utils.twisted import (
     asynchronous,
     call,
@@ -1326,6 +1327,18 @@
         RegionControllerProcess.objects.filter(
             updated__lte=remove_before_time).delete()
 
+        # Remove any processes that cannot be identified as still running
+        # on this region controller. This helps remove the need to wait
+        # 90 seconds to reap the old processes (only on region controllers
+        # where another regiond process is still running).
+        sibling_processes = (
+            RegionControllerProcess.objects
+            .filter(region=region_obj)
+            .exclude(id=self.process_id))
+        for sibling_process in sibling_processes:
+            if not is_pid_running(sibling_process.pid):
+                sibling_process.delete()
+
         # Update all endpoints for this process.
         previous_endpoint_ids = set(
             RegionControllerProcessEndpoint.objects.filter(

=== modified file 'src/maasserver/rpc/tests/test_regionservice.py'
--- src/maasserver/rpc/tests/test_regionservice.py	2016-10-28 15:58:32 +0000
+++ src/maasserver/rpc/tests/test_regionservice.py	2017-02-16 16:52:53 +0000
@@ -1424,6 +1424,34 @@
         self.assertIsNone(reload_object(old_region_process))
         self.assertIsNone(reload_object(old_other_region_process))
 
+    def test_update_removes_dead_sibling_processes(self):
+        advertising = RegionAdvertising.promote()
+
+        region = RegionController.objects.get(system_id=advertising.region_id)
+        [my_process] = region.processes.all()
+
+        running_pid = randint(1, 10)
+        running_sibling = RegionControllerProcess.objects.create(
+            region=region, pid=running_pid)
+        dead_pid = randint(11, 20)
+        dead_sibling = RegionControllerProcess.objects.create(
+            region=region, pid=dead_pid)
+
+        def is_running(pid):
+            if pid == running_pid:
+                return True
+            elif pid == dead_pid:
+                return False
+            else:
+                raise ValueError("Unknown PID.")
+
+        self.patch(regionservice, "is_pid_running").side_effect = is_running
+
+        advertising.update(self.make_addresses())
+
+        self.assertIsNotNone(reload_object(running_sibling))
+        self.assertIsNone(reload_object(dead_sibling))
+
     def test_update_updates_updated_time_on_region_and_process(self):
         current_time = now()
         self.patch(timestampedmodel, "now").return_value = current_time
@@ -1494,6 +1522,7 @@
 
         region = RegionController.objects.get(system_id=advertising.region_id)
         [process] = region.processes.all()
+        self.patch(regionservice, "is_pid_running").return_value = True
 
         # Make 3 more processes.
         for _ in range(3):

=== modified file 'src/provisioningserver/utils/ps.py'
--- src/provisioningserver/utils/ps.py	2016-03-28 13:54:47 +0000
+++ src/provisioningserver/utils/ps.py	2017-02-16 16:52:53 +0000
@@ -17,6 +17,20 @@
 )
 
 
+def is_pid_running(pid):
+    """Return True if the `pid` is running."""
+    try:
+        os.kill(pid, 0)
+    except ProcessLookupError:
+        return False
+    except PermissionError:
+        return True
+    except OSError:
+        return False
+    else:
+        return True
+
+
 def is_pid_in_container(pid, proc_path="/proc"):
     """Return True if the `pid` is running in a container.
 

=== modified file 'src/provisioningserver/utils/tests/test_ps.py'
--- src/provisioningserver/utils/tests/test_ps.py	2016-03-28 13:54:47 +0000
+++ src/provisioningserver/utils/tests/test_ps.py	2017-02-16 16:52:53 +0000
@@ -16,6 +16,7 @@
 from provisioningserver.utils.ps import (
     get_running_pids_with_command,
     is_pid_in_container,
+    is_pid_running,
     running_in_container,
 )
 from provisioningserver.utils.shell import ExternalProcessError
@@ -63,6 +64,33 @@
     """)
 
 
+class TestIsPIDRunning(MAASTestCase):
+
+    scenarios = (
+        ("running", {
+            "result": True,
+            "exception": None,
+        }),
+        ("lookup-error", {
+            "result": False,
+            "exception": ProcessLookupError(),
+        }),
+        ("permission-error", {
+            "result": True,
+            "exception": PermissionError(),
+        }),
+        ("os-error", {
+            "result": False,
+            "exception": OSError(),
+        })
+    )
+
+    def test__result(self):
+        self.patch(ps_module.os, "kill").side_effect = self.exception
+        self.assertEquals(
+            self.result, is_pid_running(random.randint(100, 200)))
+
+
 class TestIsPIDInContainer(MAASTestCase):
 
     scenarios = (