Merge lp:~robru/cupstream2distro/scan-nonpublished-silos into lp:cupstream2distro

[Robert Bruce Park]

Scan all silos every 5 minutes looking for new commits.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1163
http://jenkins.qa.ubuntu.com/job/cu2d-choo-choo-ci/833/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/cu2d-choo-choo-ci/833/rebuild

review: Approve (continuous-integration)

[lp:~robru/cupstream2distro/scan-nonpublished-silos]

1164. By Robert Bruce Park on 2015-10-21

Run unbuilt phase unconditionally.

1165. By Robert Bruce Park on 2015-10-21

Clear failures for each silo.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1165
http://jenkins.qa.ubuntu.com/job/cu2d-choo-choo-ci/834/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/cu2d-choo-choo-ci/834/rebuild

review: Approve (continuous-integration)

[lp:~robru/cupstream2distro/scan-nonpublished-silos]

1166. By Robert Bruce Park on 2015-10-21

Skip already discovered unbuilt silos.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1166
http://jenkins.qa.ubuntu.com/job/cu2d-choo-choo-ci/835/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/cu2d-choo-choo-ci/835/rebuild

review: Approve (continuous-integration)

[Robert Bruce Park (robru)]

Ok, this seems to be halfways workable, but there's one thing I'm not crazy about. There's no silo locking, so most likely this'll behave like this:

1. User pushes new commit to branch.
2. Train notices this and indicates the new commit in the status.
3. User clicks build
4. Train sees the new commit still isn't built so sets the status again, mid-build.

Although the build job does record the commit info early in the build so this might not actually be a problem in practise, but it's still a bit racy.

Would be nice to have a way of saying "actually this silo is building *RIGHT NOW* so don't scan it for new commits"

[lp:~robru/cupstream2distro/scan-nonpublished-silos]

1167. By Robert Bruce Park on 2015-10-21

Mark packages dirty on disk as well.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1167
http://jenkins.qa.ubuntu.com/job/cu2d-choo-choo-ci/836/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/cu2d-choo-choo-ci/836/rebuild

review: Approve (continuous-integration)

[lp:~robru/cupstream2distro/scan-nonpublished-silos]

1168. By Robert Bruce Park on 2015-10-21

Implement rudimentary silo locking.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1168
http://jenkins.qa.ubuntu.com/job/cu2d-choo-choo-ci/837/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/cu2d-choo-choo-ci/837/rebuild

review: Approve (continuous-integration)

[lp:~robru/cupstream2distro/scan-nonpublished-silos]

1169. By Robert Bruce Park on 2015-10-21

Set silo status better.

1170. By Robert Bruce Park on 2015-10-21

Whitespace fix.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1170
http://jenkins.qa.ubuntu.com/job/cu2d-choo-choo-ci/838/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/cu2d-choo-choo-ci/838/rebuild

review: Approve (continuous-integration)

[lp:~robru/cupstream2distro/scan-nonpublished-silos]

1171. By Robert Bruce Park on 2015-10-21

Stop checking already-dirty silos.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1171
http://jenkins.qa.ubuntu.com/job/cu2d-choo-choo-ci/839/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/cu2d-choo-choo-ci/839/rebuild

review: Approve (continuous-integration)

[Robert Bruce Park (robru)]

Alright this is looking brilliant in staging.

review: Approve

=== modified file 'citrain/migration.py'
--- citrain/migration.py	2015-09-23 22:56:07 +0000
+++ citrain/migration.py	2015-10-21 23:12:49 +0000
@@ -31,6 +31,7 @@
 
 sys.path.append(os.path.dirname(os.path.dirname(__file__)))
 
+from citrain.recipes.base import BuildBase
 from citrain.recipes.manager import Manager
 from cupstream2distro.errors import CITrainError
 from cupstream2distro.utils import env, run_script
@@ -45,14 +46,25 @@
     """
     for silo_state in SiloState.iterate():
         env.SILONAME = silo_state
-        if silo_state.is_published:
+        BuildBase.failures.clear()
+        try:
+            silo_state.enforce_lock()
+        except CITrainError as err:
+            logging.info(str(err) + '\n')
+            continue
+
+        if 'dirty' not in silo_state.tokenize():
             logging.info('Inspecting silo {}:'.format(silo_state.ppa.web_link))
+            manager = Manager(silo_state)
             try:
-                Manager(silo_state).do('migration')
-                merge_main()
+                manager.do('unbuilt')
+                if silo_state.is_published:
+                    manager.do('migration')
+                    merge_main()
             except CITrainError as err:
                 logging.info(str(err) + '\n')
-                silo_state.status = 'Migration: ' + str(err)
+                if not silo_state.mark_dirty():
+                    silo_state.status = 'Migration: ' + str(err)
                 silo_state.save_config()
     return 0
 

=== modified file 'citrain/recipes/merge.py'
--- citrain/recipes/merge.py	2015-10-20 09:03:25 +0000
+++ citrain/recipes/merge.py	2015-10-21 23:12:49 +0000
@@ -31,6 +31,7 @@
 from cupstream2distro.errors import BuildError, PrepError
 from cupstream2distro.settings import BOT_DEBFULLNAME
 from cupstream2distro.utils import env, log_value_of
+from cupstream2distro.silomanager import mark_packages_dirty
 
 
 PUBLISHABLE_STATES = ('Approved', 'Merged')
@@ -242,7 +243,9 @@
 
     def unbuilt_phase(self):
         """Block publication of merges that have new commits."""
-        if check_for_unbuilt_revids(self.name, self.merges):
+        merges = Merge.all_mps[self.name]
+        if check_for_unbuilt_revids(self.name, merges):
+            mark_packages_dirty(env.SILONAME, [self.name])
             self.failures.add(self.name + ' has new, unbuilt commits.')
 
     def merge_phase(self):

=== modified file 'cupstream2distro/silomanager.py'
--- cupstream2distro/silomanager.py	2015-10-15 00:19:37 +0000
+++ cupstream2distro/silomanager.py	2015-10-21 23:12:49 +0000
@@ -47,6 +47,7 @@
     STABLE_OVERLAY_PPA,
 )
 from cupstream2distro.utils import (
+    atomic_write,
     env,
     memoize,
     log_value_of,
@@ -196,6 +197,8 @@
         if self.strict:
             signal.signal(signal.SIGTERM, self.cleanup_on_kill)
             sys.excepthook = self.cleanup_on_exception
+            with atomic_write(self.lockfile, 'w') as lock:
+                lock.write(str(os.getpid()))
 
     def __str__(self):
         """Convert SiloState instance to printable string."""
@@ -214,6 +217,20 @@
         else:
             super().__setattr__(key, value)
 
+    @property
+    def lockfile(self):
+        """Identify where the lockfile should be."""
+        return join(SILOS_DIR, self.siloname, 'lock')
+
+    def enforce_lock(self):
+        """Raise PrepError if this silo is locked by a running process."""
+        try:
+            with utf8_open(self.lockfile) as lock:
+                os.kill(int(lock.read().strip()), 0)
+        except (FileNotFoundError, ProcessLookupError):
+            return
+        raise PrepError('Silo {} is locked by another process.'.format(self))
+
     def assign(self, requestid):
         """Assign a new requestid to an available silo."""
         self.requestid = requestid
@@ -319,6 +336,7 @@
         if packages:
             self.status = msg.format(', '.join(packages))
             self.push_to_bileto()
+        return packages
 
     def load_bileto(self):
         """Fetch request from Bileto."""

=== modified file 'tests/unit/test_recipe_merge.py'
--- tests/unit/test_recipe_merge.py	2015-10-20 08:35:12 +0000
+++ tests/unit/test_recipe_merge.py	2015-10-21 23:12:49 +0000
@@ -420,15 +420,17 @@
         self.assertFalse(merge.failures)
 
     @patch('citrain.recipes.merge.Rev')
-    def test_merge_unbuilt_phase(self, rev_mock):
+    @patch('citrain.recipes.merge.mark_packages_dirty')
+    def test_merge_unbuilt_phase(self, dirty, rev_mock):
         """Block publication if merges have new commits."""
         rev_mock.return_value.read.return_value = None
-        merge = Merge('foobaz', self.series, self.dest, self.ppa)
-        merge.merges = [Mock(
+        Merge.all_mps = dict(foobaz=[Mock(
             web_link='foo',
             source_branch=Mock(last_scanned_id='zip'),
-        )]
+        )])
+        merge = Merge('foobaz', self.series, self.dest, self.ppa)
         merge.unbuilt_phase()
+        dirty.assert_called_once_with(self.tempdir, ['foobaz'])
         self.assertEqual(merge.failures, {
             'foobaz has new, unbuilt commits.'
         })
@@ -437,11 +439,11 @@
     def test_merge_unbuilt_phase_none(self, rev_mock):
         """Allow publication if there are no new commits."""
         rev_mock.return_value.read.return_value = 'zip'
-        merge = Merge('foobaz', self.series, self.dest, self.ppa)
-        merge.merges = [Mock(
+        Merge.all_mps = dict(foobaz=[Mock(
             web_link='foo',
             source_branch=Mock(last_scanned_id='zip'),
-        )]
+        )])
+        merge = Merge('foobaz', self.series, self.dest, self.ppa)
         merge.unbuilt_phase()
         self.assertFalse(merge.failures)
 

=== modified file 'tests/unit/test_script_migration.py'
--- tests/unit/test_script_migration.py	2015-09-24 05:06:40 +0000
+++ tests/unit/test_script_migration.py	2015-10-21 23:12:49 +0000
@@ -43,17 +43,24 @@
 
     def test_main(self):
         """Trigger merges after successful migrations."""
+        tokenize = lambda: Mock(return_value={'hi'})
         states = self.script.SiloState.iterate.return_value = [
-            Mock(is_published=True) for rid in range(3)]
+            Mock(tokenize=tokenize(), is_published=True) for rid in range(3)]
         self.assertEqual(self.script.main(), 0)
         for state in states:
-            self.assertEqual(state.mock_calls, [])
+            self.assertEqual(state.mock_calls, [
+                call.enforce_lock(),
+                call.tokenize(),
+            ])
         self.assertEqual(self.script.Manager.mock_calls, [
             call(states[0]),
+            call().do('unbuilt'),
             call().do('migration'),
             call(states[1]),
+            call().do('unbuilt'),
             call().do('migration'),
             call(states[2]),
+            call().do('unbuilt'),
             call().do('migration'),
         ])
         self.assertEqual(self.script.merge_main.mock_calls, [call()] * 3)
@@ -65,12 +72,29 @@
         self.assertEqual(self.script.Manager.mock_calls, [])
         self.assertEqual(self.script.merge_main.mock_calls, [])
 
+    def test_main_skip_locked_silos(self):
+        """Skip over silos that are currently used by other processes."""
+        states = self.script.SiloState.iterate.return_value = [Mock()]
+        states[0].enforce_lock.side_effect = self.script.CITrainError
+        self.assertEqual(self.script.main(), 0)
+        self.assertEqual(self.script.Manager.mock_calls, [])
+        self.assertEqual(self.script.merge_main.mock_calls, [])
+        self.assertEqual(states[0].mock_calls, [call.enforce_lock()])
+
     def test_main_skip_unpublished_silos(self):
         """Don't merge silos that are unpublished."""
+        tokenize = Mock(return_value={'hi'})
         states = self.script.SiloState.iterate.return_value = [
-            Mock(is_published=False) for rid in range(3)]
+            Mock(tokenize=tokenize, is_published=False) for rid in range(3)]
         self.assertEqual(self.script.main(), 0)
-        self.assertEqual(self.script.Manager.mock_calls, [])
+        self.assertEqual(self.script.Manager.mock_calls, [
+            call(states[0]),
+            call().do('unbuilt'),
+            call(states[1]),
+            call().do('unbuilt'),
+            call(states[2]),
+            call().do('unbuilt'),
+        ])
         self.assertEqual(self.script.merge_main.mock_calls, [])
         for state in states:
             self.assertEqual(state.set_migrating.mock_calls, [])
@@ -78,23 +102,28 @@
 
     def test_main_dont_merge_migrating_silos(self):
         """Don't merge silos that are still migrating."""
+        tokenize = Mock(return_value={'hi'})
         states = self.script.SiloState.iterate.return_value = [
-            Mock(requestid=str(rid)) for rid in range(3)]
-        self.script.Manager.return_value.do.side_effect = MigrationError(
-            'foo is in the Proposed pocket.')
+            Mock(tokenize=tokenize, is_published=True, requestid=str(1))]
+        states[0].mark_dirty.return_value = False
+
+        def set_side_effect(*ignore):
+            """Raise an exception second time mock is called."""
+            self.script.Manager.return_value.do.side_effect = MigrationError(
+                'foo is in the Proposed pocket.')
+        self.script.Manager.return_value.do.side_effect = set_side_effect
         self.assertEqual(self.script.main(), 0)
-        for state in states:
-            self.assertEqual(
-                state.status, 'Migration: foo is in the Proposed pocket.')
-            self.assertEqual(state.mock_calls, [
-                call.save_config(),
-            ])
+        self.assertEqual(
+            states[0].status, 'Migration: foo is in the Proposed pocket.')
+        self.assertEqual(states[0].mock_calls, [
+            call.enforce_lock(),
+            call.tokenize(),
+            call.mark_dirty(),
+            call.save_config(),
+        ])
         self.assertEqual(self.script.Manager.mock_calls, [
             call(states[0]),
-            call().do('migration'),
-            call(states[1]),
-            call().do('migration'),
-            call(states[2]),
+            call().do('unbuilt'),
             call().do('migration'),
         ])
         self.assertEqual(self.script.merge_main.mock_calls, [])

=== modified file 'tests/unit/test_silomanager.py'
--- tests/unit/test_silomanager.py	2015-10-07 19:07:01 +0000
+++ tests/unit/test_silomanager.py	2015-10-21 23:12:49 +0000
@@ -120,6 +120,13 @@
         self.assertIsNot(state.info, silomanager.logging.info)
         self.assertFalse(state.strict)
 
+    def test_silostate_enforce_lock(self):
+        """Bail out when silo is locked."""
+        with self.assertRaisesRegexp(PrepError, 'locked by another'):
+            self.state.enforce_lock()
+        state = SiloState(siloname='ubuntu/landing-xyz')
+        self.assertIsNone(state.enforce_lock())
+
     @patch('cupstream2distro.silomanager.SiloState.summarize')
     @patch('cupstream2distro.silomanager.SiloState.set_ready')
     @patch('cupstream2distro.silomanager.SiloState.save_config')