The upgrade test cleanup used to conditionally remove the entire
upgrade-testsuite.dir based on test success and debug flags.
This included the base-repo source checkout with all object
files and binaries, making the CI artifact unnecessarily large.

Align the upgrade test cleanup with regular test suites where
autotest already handles per-test directory cleanup (keeping
them on failure or with -d, removing on success).

Add collect_logs() to copy the test log and per-test result
directories from base-repo/tests/ into upgrade-testsuite.dir/
so they are preserved.  Errors during collection are logged
but do not prevent cleanup from proceeding.

Add remove_base_directory() to unconditionally remove the
base-repo source checkout after logs have been collected.
The cleanup handler now simply calls both functions without
any conditional logic.

Simplify remove_upgrade_test_directory() to only handle
startup cleanup of leftovers from a previous run.

Assisted-by: Claude Opus 4.6, OpenCode
Signed-off-by: Ales Musil <[email protected]>
---
 .ci/ovn_upgrade_test.py  | 12 ++++-----
 .ci/ovn_upgrade_utils.py | 54 +++++++++++++++++++++++++++-------------
 2 files changed, 42 insertions(+), 24 deletions(-)

diff --git a/.ci/ovn_upgrade_test.py b/.ci/ovn_upgrade_test.py
index e38145cc2..3c2f7832c 100755
--- a/.ci/ovn_upgrade_test.py
+++ b/.ci/ovn_upgrade_test.py
@@ -1,7 +1,6 @@
 #!/usr/bin/env python3
 
 import atexit
-import os
 import signal
 import sys
 from pathlib import Path
@@ -15,6 +14,8 @@ from ovn_upgrade_utils import (
     ovn_upgrade_save_current_binaries,
     ovn_upgrade_extract_info,
     run_upgrade_workflow,
+    collect_logs,
+    remove_base_directory,
     remove_upgrade_test_directory,
     UpgradeConfig
 )
@@ -45,11 +46,8 @@ def main():
     test_success = False
 
     def cleanup():
-        flags = os.environ.get('TESTSUITEFLAGS', '')
-        if '-d' in flags or '--debug' in flags or not test_success:
-            log(f"Keeping {config.path.upgrade_dir} for debugging")
-        else:
-            remove_upgrade_test_directory(config)
+        collect_logs(config)
+        remove_base_directory(config)
 
     atexit.register(cleanup)
     signal.signal(signal.SIGINT, lambda s, f: sys.exit(1))
@@ -93,7 +91,7 @@ def main():
         log("UPGRADE TESTS PASSED")
     else:
         log("UPGRADE TESTS FAILED")
-        log(f"Check: {config.file.test_log}")
+        log(f"Check: {config.path.upgrade_dir}")
     log("=" * 70)
 
     return 0 if test_success else 1
diff --git a/.ci/ovn_upgrade_utils.py b/.ci/ovn_upgrade_utils.py
index b96696e1a..5804a017d 100755
--- a/.ci/ovn_upgrade_utils.py
+++ b/.ci/ovn_upgrade_utils.py
@@ -14,8 +14,7 @@ import shlex
 import sys
 
 UPGRADE_DIR = 'tests/upgrade-testsuite.dir'
-SYSTEM_TESTS_LOGS = 'tests/system-kmod-testsuite.log'
-SYSTEM_TESTS_DIR = 'tests/system-kmod-testsuite.dir'
+SYSTEM_TESTS_GLOB = 'tests/system-kmod-testsuite.*'
 BASE_REPO_DIR = 'base-repo'
 BINARIES_DIR = 'ovn-upgrade-binaries'
 BUILD_LOG = 'build-base.log'
@@ -41,13 +40,12 @@ class PathConfig:
     upgrade_dir: Path   # Path where all upgrade-tests related files are stored
     base_dir: Path      # Path for base branch i.e. from which we upgrade
     binaries_dir: Path  # Path for binaries from dst branch
-    test_dir: Path      # Path for system tests run by upgrade tests.
+    test_glob: str      # Glob for test results in base_dir
 
 
 @dataclass
 class FileConfig:
     git_log: Path
-    test_log: Path
     build_log: Path
     new_egress: Path
     m4_defines: Path
@@ -85,11 +83,10 @@ class UpgradeConfig:
             binaries_dir=upgrade_dir / BINARIES_DIR,
             base_dir=base_dir,
             upgrade_dir=upgrade_dir,
-            test_dir=base_dir / SYSTEM_TESTS_DIR,
+            test_glob=str(base_dir / SYSTEM_TESTS_GLOB),
         )
 
         file_obj = FileConfig(
-            test_log=base_dir / SYSTEM_TESTS_LOGS,
             build_log=upgrade_dir / BUILD_LOG,
             git_log=upgrade_dir / GIT_LOG,
             new_egress=upgrade_dir / NEW_EGRESS,
@@ -659,22 +656,45 @@ def run_upgrade_workflow(config):
         return True
 
 
+def collect_logs(config):
+    """Move base-repo test logs into upgrade-testsuite.dir.
+
+    Moves system-kmod-testsuite.* from base-repo/tests/ into
+    upgrade-testsuite.dir/ so they are preserved after base-repo
+    is removed.
+    """
+    upgrade_dir = config.path.upgrade_dir
+
+    if not run_shell_command(f"sudo mv {config.path.test_glob} {upgrade_dir}"):
+        log(f"Failed to collect test logs from {config.path.test_glob}")
+    else:
+        log(f"Test logs collected in {upgrade_dir}")
+
+
+def remove_base_directory(config):
+    """Remove the base-repo source checkout.
+
+    Called after collect_logs() to remove the full source tree
+    with object files and binaries that would otherwise bloat the
+    CI artifact.
+    """
+    base_dir = config.path.base_dir
+    if base_dir.exists():
+        log(f"Removing {base_dir}...")
+        run_command(f"sudo rm -rf {base_dir}")
+
+
 def remove_upgrade_test_directory(config):
+    """Remove the entire upgrade-testsuite.dir.
+
+    Used at startup to clear leftovers from a previous run.
+    """
     upgrade_dir = config.path.upgrade_dir
-    test_dir = config.path.test_dir
-    test_log = config.file.test_log
 
     if not upgrade_dir.exists():
         return True
 
     log(f"Removing old {upgrade_dir}...")
+    result = run_command(f"sudo rm -rf {upgrade_dir}")
 
-    run_command(f"sudo rm -rf {test_dir}")
-    run_command(f"sudo rm -f {test_log}")
-
-    try:
-        shutil.rmtree(upgrade_dir)
-        return True
-    except OSError as e:
-        log(f"Failed to remove {upgrade_dir}: {e}")
-        return False
+    return result.returncode == 0
-- 
2.55.0

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to