Merge lp:~free.ekanayaka/landscape-charm/dont-stop-cron into lp:~landscape/landscape-charm/trunk

[Free Ekanayaka]

Don't stop/start the cron deamon when pausing/resuming a unit. Also, wait for any running cron job to finish before running cron tests, since otherwise we could get spurious failures due to busy db locks.

Starting from r8826 the 'lsctl stop' command will also set the maintenance flag, so cron scripts won't be triggered when the unit is paused.

Don't stop/start the cron deamon when pausing/resuming a unit. Also, wait for any running cron job to finish before running cron tests, since otherwise we could get spurious failures due to busy db locks.

Starting from r8826 the 'lsctl stop' command will also set the maintenance flag, so cron scripts won't be triggered when the unit is paused.

[Björn Tillenius (bjornt)]

The bug says that we shouldn't stop the cron services and instead prevent them from running. This branch doesn't stop the cron service. What about preventing them from running?

review: Needs Information

[Free Ekanayaka (free.ekanayaka)]

Sorry, I forgot to mention that "lsctl stop" will now also prevent Landscape cron scripts from running, by creating the maintenance.txt flag file. See:

https://bugs.launchpad.net/landscape-charm/+bug/1457800

[🤖 Landscape Builder (landscape-builder)]

Command: make ci-test
Result: Success
Revno: 293
Branch: lp:~free.ekanayaka/landscape-charm/dont-stop-cron
Jenkins: https://ci.lscape.net/job/latch-test/1118/

review: Approve (test results)

[Geoff Teale (tealeg)]

+1

review: Approve

[Björn Tillenius (bjornt)]

Ok, thanks. +1 then, but please update the MP description, mentioning that the landscape-server was fixed to add the maintenance flag. It'd be good to mention the revision, since we now won't work with older version.

review: Approve

=== modified file 'lib/pause.py'
--- lib/pause.py	2015-05-07 10:26:45 +0000
+++ lib/pause.py	2015-05-29 09:28:28 +0000
@@ -15,4 +15,3 @@
 
     def _run(self):
         self._subprocess.check_call((LSCTL, "stop"))
-        self._subprocess.check_call(("service", "cron", "stop"))

=== modified file 'lib/resume.py'
--- lib/resume.py	2015-05-20 11:21:32 +0000
+++ lib/resume.py	2015-05-29 09:28:28 +0000
@@ -15,4 +15,3 @@
 
     def _run(self):
         self._subprocess.check_call((LSCTL, "start"))
-        self._subprocess.check_call(("service", "cron", "start"))

=== modified file 'lib/tests/test_pause.py'
--- lib/tests/test_pause.py	2015-05-26 11:33:50 +0000
+++ lib/tests/test_pause.py	2015-05-29 09:28:28 +0000
@@ -20,6 +20,4 @@
         """
         self.action()
         self.assertEqual(
-            [(("/usr/bin/lsctl", "stop"), {}),
-             (("service", "cron", "stop"), {})],
-            self.subprocess.calls)
+            [(("/usr/bin/lsctl", "stop"), {})], self.subprocess.calls)

=== modified file 'lib/tests/test_resume.py'
--- lib/tests/test_resume.py	2015-05-26 11:33:50 +0000
+++ lib/tests/test_resume.py	2015-05-29 09:28:28 +0000
@@ -22,6 +22,4 @@
         """
         self.action()
         self.assertEqual(
-            [(("/usr/bin/lsctl", "start"), {}),
-             (("service", "cron", "start"), {})],
-            self.subprocess.calls)
+            [(("/usr/bin/lsctl", "start"), {})], self.subprocess.calls)

=== modified file 'tests/basic/test_actions.py'
--- tests/basic/test_actions.py	2015-05-22 09:15:39 +0000
+++ tests/basic/test_actions.py	2015-05-29 09:28:28 +0000
@@ -18,16 +18,13 @@
         result = self.environment.pause_landscape()
         self.assertEqual("completed", result["status"])
         service_status = self.environment.get_landscape_services_status()
-        # All Landcape services and cron have been stopped
+        # All Landcape services have been stopped
         self.assertEqual([], service_status["running"])
         self.assertTrue(len(service_status["stopped"]) > 0)
-        self.assertFalse(
-            self.environment.is_cron_running("landscape-server/0"))
 
         result = self.environment.resume_landscape()
         self.assertEqual("completed", result["status"])
         service_status = self.environment.get_landscape_services_status()
-        # All Landcape services and cron have been started
+        # All Landcape services have been started
         self.assertEqual([], service_status["stopped"])
         self.assertTrue(len(service_status["running"]) > 0)
-        self.assertTrue(self.environment.is_cron_running("landscape-server/0"))

=== modified file 'tests/basic/test_service.py'
--- tests/basic/test_service.py	2015-05-22 09:15:39 +0000
+++ tests/basic/test_service.py	2015-05-29 09:28:28 +0000
@@ -11,7 +11,7 @@
 from subprocess import check_output, CalledProcessError, PIPE
 
 from helpers import IntegrationTest
-from layers import OneLandscapeUnitLayer
+from layers import OneLandscapeUnitLayer, OneLandscapeUnitNoCronLayer
 
 
 class ServiceTest(IntegrationTest):
@@ -148,18 +148,10 @@
     the cron daemon will be stopped, so Landscape cron jobs in particular
     won't be run.
     """
-    layer = OneLandscapeUnitLayer
+    layer = OneLandscapeUnitNoCronLayer
 
     cron_unit = "landscape-server/0"
 
-    @classmethod
-    def setUpClass(cls):
-        cls._stop_cron(cls.cron_unit)
-
-    @classmethod
-    def tearDownClass(cls):
-        cls._start_cron(cls.cron_unit)
-
     def _sanitize_ssh_output(self, output,
                              remove_text=["sudo: unable to resolve",
                                           "Warning: Permanently added"]):
@@ -265,13 +257,3 @@
         frontend = self.environment.get_haproxy_public_address()
         self.assertEqual(
             "https://%s/" % frontend, config["global"]["root-url"])
-
-    @staticmethod
-    def _stop_cron(unit):
-        cmd = ["juju", "ssh", unit, "sudo", "service", "cron", "stop", "2>&1"]
-        check_output(cmd, stderr=PIPE)
-
-    @staticmethod
-    def _start_cron(unit):
-        cmd = ["juju", "ssh", unit, "sudo", "service", "cron", "start", "2>&1"]
-        check_output(cmd, stderr=PIPE)

=== modified file 'tests/helpers.py'
--- tests/helpers.py	2015-05-21 11:02:48 +0000
+++ tests/helpers.py	2015-05-29 09:28:28 +0000
@@ -168,13 +168,25 @@
         action_id = self._do_action("resume", "landscape-server/" + str(unit))
         return self._fetch_action(action_id)
 
-    def stop_landscape_service(self, service, unit=0):
-        """Stop the given Landscape service on the given unit.
-
-        The service being stopped will be automatically restarted upon cleanUp.
+    def wait_landscape_cron_jobs(self, unit=0):
+        """Wait for running cron jobs to finish on the given Landscape unit."""
+        unit = self._deployment.sentry.unit["landscape-server/%d" % unit]
+        output, code = unit.run(
+            "sudo /opt/canonical/landscape/wait-batch-scripts")
+        if code != 0:
+            raise RuntimeError(output)
+
+    def stop_landscape_service(self, service, unit=0, restore=True):
+        """Stop the given service on the given Landscape unit.
+
+        @param service: The service to stop.
+        @param unit: The Landscape unit to act on.
+        @param restore: Whether the service should be automatically restarted
+            upon cleanUp.
         """
         self._control_landscape_service("stop", service, unit)
-        self._stopped_landscape_services.append((service, unit))
+        if restore:
+            self._stopped_landscape_services.append((service, unit))
 
     def start_landscape_service(self, service, unit=0):
         """Start the given Landscape service on the given unit."""
@@ -210,18 +222,6 @@
             service_status["stopped"].append(service_name)
         return service_status
 
-    def is_cron_running(self, unit):
-        """Returns whether cron is running on a unit."""
-        unit_sentry = self._deployment.sentry.unit[unit]
-        output, code = unit_sentry.run("service cron status")
-        assert code == 0
-        if "stop/waiting" in output:
-            return False
-        elif "start/running" in output:
-            return True
-        else:
-            raise AssertionError("Malformed status output: " + output)
-
     def configure_ssl(self, cert, key):
         """Start the given Landscape service on the given unit."""
         self._deployment.configure(

=== modified file 'tests/layers.py'
--- tests/layers.py	2015-05-13 17:57:51 +0000
+++ tests/layers.py	2015-05-29 09:28:28 +0000
@@ -15,10 +15,28 @@
         cls.environment.setUp()
 
     @classmethod
-    def tearDown(cls):
+    def testTearDown(cls):
         cls.environment.cleanUp()
 
 
+class OneLandscapeUnitNoCronLayer(OneLandscapeUnitLayer):
+    """Layer for all tests needing to run with the cron daemon stopped.
+
+    The deployment has the same structure as OneLandscapeUnitLayer, the cron
+    daemon will be stopped. Also, the layer setup waits for any currently
+    running Landscape cron job to finish.
+    """
+
+    @classmethod
+    def setUp(cls):
+        cls.environment.stop_landscape_service("cron", restore=False)
+        cls.environment.wait_landscape_cron_jobs()
+
+    @classmethod
+    def tearDown(cls):
+        cls.environment.start_landscape_service("cron")
+
+
 class OneLandscapeUnitCustomSSLCertificateLayer(OneLandscapeUnitLayer):
     """Layer for all tests needing a deployment with a custom SSL certificate.