[QC-1262] repo_cleaner instances must not share their last execution time

Framework/script/RepoCleaner/ReleaseNotes.md

@@ -1,7 +1,7 @@
 # Release notes
 
-New
-- 
+1.11
+- [QC-1262] - different repo_cleaner must use a different file for the last run
 
 1.10
 - Revive and clean up repocleaner tests

Framework/script/RepoCleaner/qcrepocleaner/o2-qc-repo-cleaner

@@ -23,6 +23,7 @@ import argparse
 import importlib
 import logging
 import multiprocessing as mp
+import os
 import re
 import socket
 import sys
@@ -106,6 +107,8 @@ def parse_args():
                         help='Do not check when was the last execution, run from timestamp 0.')
     parser.add_argument('--print-versions-details', dest='print_versions_details', action='store_true', default=False,
                         help='Print extra details about the versions if enabled..')
+    parser.add_argument('--instance-name', dest='instance_name', action='store', default=None,
+                        help='Instance name used to differentiate execution state files (timestamp and PID). If omitted, inferred from configuration.')
     args = parser.parse_args()
     dryable.set(args.dry_run)
     logging.info(args)
@@ -215,42 +218,79 @@ def find_matching_rules(rules, object_path):
     return result
 
 
-filepath = tempfile.gettempdir() + "/repoCleaner.txt"
+def get_instance_name(args=None):
+    """
+    Determine the instance identifier based on args or config.
+    """
+    if args is None:
+        return "default"
+    if getattr(args, 'instance_name', None):
+        return args.instance_name
+    if getattr(args, 'config_consul', None):
+        items = args.config_consul.split(':')
+        if len(items) >= 3:
+            return Path(items[2]).stem
+    if getattr(args, 'config_git', None):
+        return "git"
+    if getattr(args, 'config', None):
+        config_stem = Path(args.config).stem
+        if getattr(args, 'only_path', None) and args.only_path != "":
+            sanitized_path = re.sub(r'[^a-zA-Z0-9_-]', '_', args.only_path.strip('/'))
+            return f"{config_stem}_{sanitized_path}"
+        return config_stem
+    return "default"
+
+
+def get_timestamp_filepath(instance_name: str = None) -> str:
+    """
+    Returns the path to the file where the timestamp of the last execution is stored for a given instance.
+    """
+    if instance_name:
+        sanitized = re.sub(r'[^a-zA-Z0-9_-]', '_', instance_name)
+        return os.path.join(tempfile.gettempdir(), f"repoCleaner_{sanitized}.txt")
+    else:
+        return os.path.join(tempfile.gettempdir(), "repoCleaner.txt")
+
+
+filepath = get_timestamp_filepath()
 currentTimeStamp = int(time.time() * 1000)
 
 
-def get_ts_last_execution(ignore_last_execution: bool):
+def get_ts_last_execution(ignore_last_execution: bool, filepath_override: str = None):
     """
     Returns the timestamp of the last execution.
-    It is stored in a file in $TMP/repoCleaner.txt.
+    It is stored in a file in $TMP/repoCleaner_<instance>.txt.
     :return: the timestamp of the last execution or 0 if it cannot find it.
     """
     if ignore_last_execution:
         logging.info(f"Option ignore_last_execution set, we return 0 as timestamp.")
         return 0
 
+    target_filepath = filepath_override if filepath_override is not None else filepath
     try:
-        f = open(filepath, "r")
+        f = open(target_filepath, "r")
     except IOError:
-        logging.info(f"File {filepath} not readable, we return 0 as timestamp.")
+        logging.info(f"File {target_filepath} not readable, we return 0 as timestamp.")
         return 0
     timestamp = f.read()
-    logging.info(f"Timestamp retrieved from {filepath}: {timestamp}")
+    logging.info(f"Timestamp retrieved from {target_filepath}: {timestamp}")
     f.close()
     return timestamp
 
 
-def store_saved_timestamp():
+def store_saved_timestamp(filepath_override: str = None, timestamp: int = None):
     """
     Store the timestamp we saved at the beginning of the execution of this script.
     """
+    target_filepath = filepath_override if filepath_override is not None else filepath
+    ts = timestamp if timestamp is not None else currentTimeStamp
     try:
-        f = open(filepath, "w+")
+        f = open(target_filepath, "w+")
     except IOError:
-        logging.error(f"Could not write the saved timestamp to {filepath}")
+        logging.error(f"Could not write the saved timestamp to {target_filepath}")
         return
-    f.write(str(currentTimeStamp))
-    logging.info(f"Stored timestamp {currentTimeStamp} in {filepath}")
+    f.write(str(ts))
+    logging.info(f"Stored timestamp {ts} in {target_filepath}")
     f.close()
 
 
@@ -376,13 +416,15 @@ def process_object(object_path, rules, ccdb, args):
 
 
 def run(args, ccdb_url, rules):
+    instance_name = get_instance_name(args)
+    ts_filepath = get_timestamp_filepath(instance_name)
 
     # Get list of objects from CCDB
     ccdb = Ccdb(ccdb_url, args.print_versions_details)
     ccdb.logger = logging.getLogger
     ccdb.set_adjustable_eov = args.set_adjustableEOV
     logging.info(f"ccdb.set_adjustable_eov: {ccdb.set_adjustable_eov}")
-    paths = ccdb.get_objects_list(get_ts_last_execution(args.ignore_last_execution), args.only_path, args.only_path_no_subdir)
+    paths = ccdb.get_objects_list(get_ts_last_execution(args.ignore_last_execution, ts_filepath), args.only_path, args.only_path_no_subdir)
     if args.only_path != '':
         paths = [item for item in paths if item is not None and item.startswith(args.only_path)]
     logging.debug(paths)
@@ -399,7 +441,7 @@ def run(args, ccdb_url, rules):
     message = datetime.today().strftime('%Y-%m-%d-%H:%M:%S')
     store_crucial_log(message + f" - End of the cleaner (total deleted: {ccdb.counter_deleted}, total updated: {ccdb.counter_validity_updated})")
     if not args.dry_run and not args.ignore_last_execution:
-        store_saved_timestamp()
+        store_saved_timestamp(ts_filepath)
 
 # ****************
 # We start here !
@@ -413,8 +455,11 @@ def main():
     args = parse_args()
     logging.getLogger().setLevel(int(args.log_level))
 
+    instance_name = get_instance_name(args)
+    pid_filename = f'o2-qc-repo-cleaner_{instance_name}.pid' if instance_name else 'o2-qc-repo-cleaner.pid'
+
     try:
-        with PIDFile(filename='o2-qc-repo-cleaner.pid'):
+        with PIDFile(filename=pid_filename):
             ccdb_url, rules = read_config(args)
             run(args, ccdb_url, rules)
     except AlreadyRunningError:

Framework/script/RepoCleaner/tests/test_repoCleaner.py

@@ -21,16 +21,23 @@ def import_path(path):  # needed because o2-qc-repo-cleaner has no suffix
     sys.modules[module_name] = module
     return module
 
-repoCleaner = import_path("../qcrepocleaner/o2-qc-repo-cleaner")
+script_dir = os.path.dirname(os.path.abspath(__file__))
+script_path = os.path.join(script_dir, "../qcrepocleaner/o2-qc-repo-cleaner")
+repoCleaner = import_path(script_path)
 parse_config = repoCleaner.parse_config
 Rule = repoCleaner.Rule
 find_matching_rules = repoCleaner.find_matching_rules
+get_instance_name = repoCleaner.get_instance_name
+get_timestamp_filepath = repoCleaner.get_timestamp_filepath
+get_ts_last_execution = repoCleaner.get_ts_last_execution
+store_saved_timestamp = repoCleaner.store_saved_timestamp
 
 
 class TestRepoCleaner(unittest.TestCase):
 
     def test_parseConfig(self):
-        args = parse_config("config-test.yaml")
+        config_test_path = os.path.join(script_dir, "config-test.yaml")
+        args = parse_config(config_test_path)
         self.assertEqual(args['ccdb_url'], CCDB_TEST_URL)
         rules = args['rules']
         self.assertEqual(len(rules), 3)
@@ -70,6 +77,80 @@ def test_findMatchingRule(self):
         rules.append(Rule('.*', '0', 'policyAll'))
         self.assertEqual(find_matching_rules(rules, 'task3/obj1')[0].policy, 'policyAll')
 
+    def test_get_instance_name(self):
+        class DummyArgs:
+            def __init__(self, **kwargs):
+                self.__dict__.update(kwargs)
+
+        self.assertEqual(get_instance_name(None), 'default')
+        self.assertEqual(get_instance_name(DummyArgs()), 'default')
+
+        # explicit instance name
+        args = DummyArgs(instance_name='custom_inst', config='config.yaml')
+        self.assertEqual(get_instance_name(args), 'custom_inst')
+
+        # config file
+        args = DummyArgs(config='/path/to/my_cleaner_config.yaml')
+        self.assertEqual(get_instance_name(args), 'my_cleaner_config')
+
+        # config with only_path
+        args = DummyArgs(config='config.yaml', only_path='qc/ITS/MO')
+        self.assertEqual(get_instance_name(args), 'config_qc_ITS_MO')
+
+        # consul config
+        args = DummyArgs(config_consul='localhost:8500:cleaner-tpc.yaml')
+        self.assertEqual(get_instance_name(args), 'cleaner-tpc')
+
+        # git config
+        args = DummyArgs(config_git=True)
+        self.assertEqual(get_instance_name(args), 'git')
+
+    def test_get_timestamp_filepath(self):
+        default_path = get_timestamp_filepath()
+        self.assertTrue(default_path.endswith('repoCleaner.txt'))
+
+        inst_path = get_timestamp_filepath('custom_inst')
+        self.assertTrue(inst_path.endswith('repoCleaner_custom_inst.txt'))
+        self.assertNotEqual(default_path, inst_path)
+
+    def test_instance_timestamp_isolation(self):
+        inst1_file = get_timestamp_filepath('instance_1_test')
+        inst2_file = get_timestamp_filepath('instance_2_test')
+
+        # Cleanup if leftover
+        for f in [inst1_file, inst2_file]:
+            if os.path.exists(f):
+                os.remove(f)
+
+        try:
+            # Initially no execution timestamp
+            self.assertEqual(get_ts_last_execution(False, inst1_file), 0)
+            self.assertEqual(get_ts_last_execution(False, inst2_file), 0)
+
+            # Store timestamp for instance 1
+            ts1 = 1600000000000
+            store_saved_timestamp(inst1_file, timestamp=ts1)
+
+            # Instance 1 has ts1, instance 2 is still 0
+            self.assertEqual(int(get_ts_last_execution(False, inst1_file)), ts1)
+            self.assertEqual(get_ts_last_execution(False, inst2_file), 0)
+
+            # Store timestamp for instance 2
+            ts2 = 1700000000000
+            store_saved_timestamp(inst2_file, timestamp=ts2)
+
+            # Both retain their own distinct timestamps
+            self.assertEqual(int(get_ts_last_execution(False, inst1_file)), ts1)
+            self.assertEqual(int(get_ts_last_execution(False, inst2_file)), ts2)
+
+            # Test ignore_last_execution option
+            self.assertEqual(get_ts_last_execution(True, inst1_file), 0)
+            self.assertEqual(get_ts_last_execution(True, inst2_file), 0)
+        finally:
+            for f in [inst1_file, inst2_file]:
+                if os.path.exists(f):
+                    os.remove(f)
+
 
 if __name__ == '__main__':
     unittest.main()