[runner] more robust resiliency

support broken symlink and don't care about modified files that wouldn't be transferred anyway.

/cc @jm for python review :)
/cc @luke @Nicolas to be sure it's what we want

/reviewed-on nexedi/slapos.toolbox!55

slapos/agent/agent.py

@@ -70,6 +70,9 @@ class AutoSTemp(object):
 from tester import SoftwareReleaseTester
 
 class TestMap(object):
+  # tell pytest to skip this class (even if name starts with Test)
+  __test__ = False
+
   def __init__(self, test_dict):
     self.ran_test_set = set()
     self.test_map_dict = collections.OrderedDict()

slapos/resilient/runner_exporter.py

@@ -96,12 +96,41 @@ def synchroniseRunnerWorkingDirectory(config, backup_path):
     )
 
 
-def getBackupFilesModifiedDuringExportList(export_start_date):
+def getBackupFilesModifiedDuringExportList(config, export_start_date):
   export_time = time.time() - export_start_date
-  return subprocess.check_output((
-      'find', '-cmin',  str(export_time / 60.), '-type', 'f', '-path', '*/srv/backup/*'
-    )).split()
+  # find all files that were modified during export
+  modified_files = subprocess.check_output((
+      'find', 'instance', '-cmin',  str(export_time / 60.), '-type', 'f', '-path', '*/srv/backup/*'
+    ))
+  if not modified_files:
+    return ()
+
+  # filter those modified files through rsync --exclude getExcludePathList.
+  # Indeed, some modified files may be listed in getExcludePathList and in this
+  # case, we won't copy them to PBS so it's not really important if they are
+  # modified.
+  rsync_arg_list = [
+    config.rsync_binary,
+    '-n',
+    '--out-format=%n',
+    '--files-from=-',
+    '--relative',
+    '--no-implied-dirs'
+  ]
+  rsync_arg_list += map("--exclude={}".format, getExcludePathList(os.getcwd()))
+  rsync_arg_list += '.', 'unexisting_dir_or_file_just_to_have_the_output'
+  process = subprocess.Popen(rsync_arg_list, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
+  output = process.communicate(modified_files)[0]
+  retcode = process.poll()
+  if retcode:
+      raise CalledProcessError(retcode, rsync_arg_list[0], output=output)
+
+  important_modified_file_list = output.splitlines()
+  not_important_modified_file_set = set(modified_files.splitlines()).difference(important_modified_file_list)
+  if not_important_modified_file_set:
+    print("WARNING: The following files in srv/backup were modified since the exporter started (srv/backup should contain almost static files):", *sorted(not_important_modified_file_set), sep='\n')
 
+  return important_modified_file_list
 
 def runExport():
   export_start_date = int(time.time())
@@ -146,11 +175,11 @@ def runExport():
   time.sleep(10)
 
   # Check that export didn't happen during backup of instances
-  with CwdContextManager(os.path.join(runner_working_path, 'instance')):
-    modified_file_list = getBackupFilesModifiedDuringExportList(export_start_date)
+  with CwdContextManager(runner_working_path):
+    modified_file_list = getBackupFilesModifiedDuringExportList(args, export_start_date)
     if len(modified_file_list):
-      print("ERROR: Files were modified since the backup started, exporter should be re-run."
-            " Let's sleep %s minutes, to let the backup end. Modified files:\n%s" % (
+      print("ERROR: The following files in srv/backup were modified since the exporter started. Since they must be backup, exporter should be re-run."
+            " Let's sleep %s minutes, to let the backup end.\n%s" % (
               args.backup_wait_time, '\n'.join(modified_file_list)))
       time.sleep(args.backup_wait_time * 60)
       sys.exit(1)

slapos/resilient/runner_utils.py

@@ -124,9 +124,12 @@ def writeSignatureFile(slappart_signature_method_dict, runner_working_path, sign
         )
         break
 
+    # construct list of file path and remove broken symlink
+    filepath_list = filter(os.path.isfile, [os.path.join(dirpath, filename) for filename in filename_list])
+
     if signature_process:
       (output, error_output) = signature_process.communicate(
-        '\0'.join([os.path.join(dirpath, filename) for filename in filename_list])
+        '\0'.join(filepath_list)
       )
 
       if signature_process.returncode != 0:
@@ -143,10 +146,7 @@ def writeSignatureFile(slappart_signature_method_dict, runner_working_path, sign
       signature_list.extend(output.strip('\n').split('\n'))
     else:
       signature_list.extend(
-        getSha256Sum([
-          os.path.join(dirpath, filename)
-          for filename in filename_list
-        ])
+        getSha256Sum(filepath_list)
       )
 
   # Write the signatures in file

slapos/test/test_runner_exporter.py

@@ -40,7 +40,15 @@ __buildout_signature__ = MarkupSafe-1.0-py2.7-linux-x86_64.egg Jinja2-2.10-py2.7
 recipe = slapos.recipe.template:jinja2
 rendered = /some/prefix/slappart18/test/srv/exporter.exclude
 template = inline:
-        srv/backup/**"""
+        srv/backup/*.log
+
+[exclude1]
+__buildout_installed__ = {cwd}/instance/slappart1/srv/exporter.exclude
+__buildout_signature__ = MarkupSafe-1.0-py2.7-linux-x86_64.egg Jinja2-2.10-py2.7.egg zc.buildout-2.12.2-py2.7.egg slapos.recipe.template-4.3-py2.7.egg setuptools-40.4.3-py2.7.egg
+recipe = slapos.recipe.template:jinja2
+rendered = /some/prefix/slappart18/test/srv/exporter.exclude
+template = inline:
+        srv/backup/log/**"""
 
 
 class Config():
@@ -76,24 +84,37 @@ class TestRunnerExporter(unittest.TestCase):
 
     """Create data mirroring tested_instance_cfg"""
     os.makedirs('instance/slappart0/etc')
-    os.makedirs('instance/slappart0/srv/backup')
+    os.makedirs('instance/slappart0/srv/backup/important_logs')
 
     os.makedirs('instance/slappart1/etc')
-    os.makedirs('instance/slappart1/srv/backup')
+    os.makedirs('instance/slappart1/srv/backup/log')
 
     self._createFile('instance/slappart0/.installed.cfg',
                      tested_instance_cfg.format(cwd=os.getcwd()))
 
     self._createFile('instance/slappart0/srv/backup/data.dat',
                      'all my fortune lays on this secret !')
+    self._createFile('instance/slappart0/srv/backup/important_logs/this_is_a.log',
+                     'this log is very important !')
+    self._createFile('instance/slappart0/srv/backup/data.log',
+                     'this log is not so important !')
     self._createFile('instance/slappart0/srv/exporter.exclude',
-                     'srv/backup/**')
+                     'srv/backup/*.log')
 
     self._createFile('instance/slappart0/etc/config.json')
     self._createFile('instance/slappart0/etc/.parameters.xml')
     self._createFile('instance/slappart0/etc/.project',
                      'workspace/slapos-dev/software/erp5')
 
+    self._createFile('instance/slappart1/srv/backup/data.dat',
+                     'This is important data also !')
+    self._createFile('instance/slappart1/srv/backup/log/log1',
+                     'First log')
+    self._createFile('instance/slappart1/srv/backup/log/log2',
+                     'Second log')
+    self._createFile('instance/slappart1/srv/exporter.exclude',
+                     'srv/backup/log/**')
+
     self._createExecutableFile(
       'instance/slappart1/srv/.backup_identity_script',
       "#!/bin/sh\nexec xargs -0 md5sum"
@@ -120,8 +141,10 @@ class TestRunnerExporter(unittest.TestCase):
         '.installed*.cfg',
         'instance/slappart0/etc/nicolas.txt',
         'instance/slappart0/etc/rafael.txt',
-        'instance/slappart0/srv/backup/**',
+        'instance/slappart0/srv/backup/*.log',
         'instance/slappart0/srv/exporter.exclude',
+        'instance/slappart1/srv/backup/log/**',
+        'instance/slappart1/srv/exporter.exclude',
       ]
     )
 
@@ -155,7 +178,7 @@ class TestRunnerExporter(unittest.TestCase):
 
     self.assertEqual(check_output_mock.call_count, 1)
     check_output_mock.assert_any_call(
-      ['rsync', '-rlptgov', '--stats', '--safe-links', '--ignore-missing-args', '--delete', '--delete-excluded', '--exclude=*.pid', '--exclude=*.sock', '--exclude=*.socket', '--exclude=.installed*.cfg', '--exclude=instance/slappart0/etc/nicolas.txt', '--exclude=instance/slappart0/etc/rafael.txt', '--exclude=instance/slappart0/srv/backup/**', '--exclude=instance/slappart0/srv/exporter.exclude', 'instance', 'project', 'proxy.db', 'public', 'backup/runner/runner']
+      ['rsync', '-rlptgov', '--stats', '--safe-links', '--ignore-missing-args', '--delete', '--delete-excluded', '--exclude=*.pid', '--exclude=*.sock', '--exclude=*.socket', '--exclude=.installed*.cfg', '--exclude=instance/slappart0/etc/nicolas.txt', '--exclude=instance/slappart0/etc/rafael.txt', '--exclude=instance/slappart0/srv/backup/*.log', '--exclude=instance/slappart0/srv/exporter.exclude', '--exclude=instance/slappart1/srv/backup/log/**', '--exclude=instance/slappart1/srv/exporter.exclude', 'instance', 'project', 'proxy.db', 'public', 'backup/runner/runner']
     )
 
   def test_getSlappartSignatureMethodDict(self):
@@ -180,6 +203,10 @@ class TestRunnerExporter(unittest.TestCase):
     self._createFile('backup/runner/instance/slappart0/data', 'hello')
     self._createFile('backup/runner/instance/slappart1/data', 'world')
 
+    os.symlink('data', 'backup/runner/instance/slappart0/good_link')
+    os.symlink(os.path.abspath('backup/runner/instance/slappart0/data'), 'backup/runner/instance/slappart0/good_abs_link')
+    os.symlink('unexisting_file', 'backup/runner/instance/slappart0/bad_link')
+
     slappart_signature_method_dict = {
       './instance/slappart1': './instance/slappart1/srv/.backup_identity_script',
     }
@@ -191,24 +218,30 @@ class TestRunnerExporter(unittest.TestCase):
         signature_file_content = f.read()
 
     # Slappart1 is using md5sum as signature, others are using sha256sum (default)
-    self.assertEqual(signature_file_content, """2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824  ./runner/instance/slappart0/data
+    self.assertEqual(signature_file_content,
+       """2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824  ./runner/instance/slappart0/data
+2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824  ./runner/instance/slappart0/good_abs_link
+2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824  ./runner/instance/slappart0/good_link
 49b74873d57ff0307b7c9364e2fe2a3876d8722fbe7ce3a6f1438d47647a86f4  ./etc/.project
 7d793037a0760186574b0282f2f435e7  ./runner/instance/slappart1/data""")
 
   def test_getBackupFilesModifiedDuringExportList(self):
     self._setUpFakeInstanceFolder()
-    with runner_exporter.CwdContextManager('instance'):
-      self.assertEqual(
-        runner_exporter.getBackupFilesModifiedDuringExportList(time.time() - 5),
-        ['./slappart0/srv/backup/data.dat']
-      )
-      time.sleep(2)
-      self.assertEqual(
-        runner_exporter.getBackupFilesModifiedDuringExportList(time.time() - 1),
-        []
-      )
-      self._createFile('slappart1/srv/backup/bakckup.data', 'my backup')
-      self.assertEqual(
-        runner_exporter.getBackupFilesModifiedDuringExportList(time.time() - 1),
-        ['./slappart1/srv/backup/bakckup.data']
-      )
+    config = Config()
+    config.rsync_binary = 'rsync'
+
+    self.assertEqual(
+      runner_exporter.getBackupFilesModifiedDuringExportList(config, time.time() - 5),
+      ['instance/slappart0/srv/backup/data.dat',
+       'instance/slappart0/srv/backup/important_logs/this_is_a.log',
+       'instance/slappart1/srv/backup/data.dat']
+    )
+    time.sleep(2)
+    self.assertFalse(
+      runner_exporter.getBackupFilesModifiedDuringExportList(config, time.time() - 1)
+    )
+    self._createFile('instance/slappart1/srv/backup/bakckup.data', 'my backup')
+    self.assertEqual(
+      runner_exporter.getBackupFilesModifiedDuringExportList(config, time.time() - 1),
+      ['instance/slappart1/srv/backup/bakckup.data']
+    )