deletion is handled acccording to pattern mentioned in config accross directories

Author: Avinash Singh

config/config.yaml.example

@@ -20,6 +20,10 @@ vehicle_id: "vehicle-CN-001"
 ## ============================================
 # LOG DIRECTORIES TO MONITOR
 # ============================================
+# ⚠️ CRITICAL: When monitoring /var/log/syslog, you MUST use pattern "syslog.[1-9]*"
+#    to avoid uploading the active syslog file. Uploading the active file creates
+#    an infinite loop where the service continuously re-uploads the file it just wrote to.
+#
 # Four log sources are monitored with explicit source names:
 # 1. Terminal logs: terminal/ in S3
 # 2. ROS logs: ros/ in S3
@@ -37,7 +41,7 @@ vehicle_id: "vehicle-CN-001"
 #   - If pattern is omitted, ALL files in the directory are uploaded
 #   - Supports wildcards: * (any characters), ? (single character)
 #   - Examples:
-#       pattern: "syslog*"       → Matches: syslog, syslog.1, syslog.2.gz
+#       pattern: "syslog.[1-9]*" → Matches: syslog.1, syslog.2.gz (skips active syslog)
 #       pattern: "*.log"         → Matches: error.log, debug.log
 #       pattern: "test_*.mcap"   → Matches: test_2024.mcap, test_run1.mcap
 #
@@ -67,7 +71,8 @@ log_directories:
 
   - path: /var/log
     source: syslog
-    pattern: "syslog.[1-9]*"  # Only rotated files, NOT active syslog1, etc
+    pattern: "syslog.[1-9]*"  # CRITICAL: Only rotated files (syslog.1, syslog.2.gz)
+                               # Skips /var/log/syslog (active file - would cause infinite loop!)
     recursive: false     # Do NOT monitor subdirectories (system logs have many folders)
 
   - path: ${HOME}/ros2_ws/log

scripts/deployment/health_check.sh

@@ -188,6 +188,14 @@ fi
 print_section "S3 Connectivity"
 
 if command -v aws &> /dev/null; then
+    # Detect if running with sudo and use actual user's AWS credentials
+    if [ -n "$SUDO_USER" ] && [ "$SUDO_USER" != "root" ]; then
+        # Running with sudo - use actual user's home for credentials
+        REAL_HOME=$(eval echo ~$SUDO_USER)
+        export AWS_SHARED_CREDENTIALS_FILE="$REAL_HOME/.aws/credentials"
+        export AWS_CONFIG_FILE="$REAL_HOME/.aws/config"
+    fi
+
     if [ -n "$AWS_PROFILE" ]; then
         AWS_CMD="aws --profile $AWS_PROFILE --region $AWS_REGION"
     else
@@ -197,35 +205,43 @@ if command -v aws &> /dev/null; then
     # Check recent uploads
     info "Checking recent uploads..."
 
-    RECENT_UPLOADS=$($AWS_CMD s3 ls "s3://$S3_BUCKET/$VEHICLE_ID/" --recursive 2>/dev/null | tail -10)
-
-    if [ -n "$RECENT_UPLOADS" ]; then
-        check_pass "S3 bucket accessible"
-
-        TOTAL_FILES=$($AWS_CMD s3 ls "s3://$S3_BUCKET/$VEHICLE_ID/" --recursive 2>/dev/null | wc -l)
-        check_pass "Total files in S3: $TOTAL_FILES"
-
-        echo ""
-        echo -e "${BOLD}Latest Uploads:${NC}"
-        echo "$RECENT_UPLOADS" | tail -5 | while read -r line; do
-            FILE_DATE=$(echo "$line" | awk '{print $1, $2}')
-            FILE_SIZE=$(echo "$line" | awk '{print $3}')
-            FILE_NAME=$(echo "$line" | awk '{print $4}' | rev | cut -d'/' -f1 | rev)
-
-            # Convert size to human readable
-            if [ "$FILE_SIZE" -gt 1048576 ]; then
-                FILE_SIZE_H="$(echo "scale=1; $FILE_SIZE / 1048576" | bc) MB"
-            elif [ "$FILE_SIZE" -gt 1024 ]; then
-                FILE_SIZE_H="$(echo "scale=1; $FILE_SIZE / 1024" | bc) KB"
-            else
-                FILE_SIZE_H="${FILE_SIZE} B"
-            fi
-
-            echo -e "  ${CYAN}$FILE_DATE${NC} - $FILE_NAME (${FILE_SIZE_H})"
-        done
+    # Test AWS credentials first
+    AWS_TEST_OUTPUT=$($AWS_CMD sts get-caller-identity 2>&1)
+    if [ $? -ne 0 ]; then
+        check_warn "Cannot access AWS (credentials issue)"
+        info "Error: $(echo "$AWS_TEST_OUTPUT" | head -1)"
+        info "Make sure AWS credentials are configured for profile: $AWS_PROFILE"
     else
-        check_warn "No files found in S3 yet"
-        info "Files will appear after first upload"
+        RECENT_UPLOADS=$($AWS_CMD s3 ls "s3://$S3_BUCKET/$VEHICLE_ID/" --recursive 2>/dev/null | tail -10)
+
+        if [ -n "$RECENT_UPLOADS" ]; then
+            check_pass "S3 bucket accessible"
+
+            TOTAL_FILES=$($AWS_CMD s3 ls "s3://$S3_BUCKET/$VEHICLE_ID/" --recursive 2>/dev/null | wc -l)
+            check_pass "Total files in S3: $TOTAL_FILES"
+
+            echo ""
+            echo -e "${BOLD}Latest Uploads:${NC}"
+            echo "$RECENT_UPLOADS" | tail -5 | while read -r line; do
+                FILE_DATE=$(echo "$line" | awk '{print $1, $2}')
+                FILE_SIZE=$(echo "$line" | awk '{print $3}')
+                FILE_NAME=$(echo "$line" | awk '{print $4}' | rev | cut -d'/' -f1 | rev)
+
+                # Convert size to human readable
+                if [ "$FILE_SIZE" -gt 1048576 ]; then
+                    FILE_SIZE_H="$(echo "scale=1; $FILE_SIZE / 1048576" | bc) MB"
+                elif [ "$FILE_SIZE" -gt 1024 ]; then
+                    FILE_SIZE_H="$(echo "scale=1; $FILE_SIZE / 1024" | bc) KB"
+                else
+                    FILE_SIZE_H="${FILE_SIZE} B"
+                fi
+
+                echo -e "  ${CYAN}$FILE_DATE${NC} - $FILE_NAME (${FILE_SIZE_H})"
+            done
+        else
+            check_warn "No files found in S3 yet"
+            info "Files will appear after first upload"
+        fi
     fi
 else
     check_warn "AWS CLI not available for S3 verification"

scripts/deployment/verify_deployment.sh

@@ -240,8 +240,9 @@ if [ -z "$LOG_DIRS" ]; then
     check_warn "No log directories configured"
 else
     for dir in $LOG_DIRS; do
-        # Replace USER placeholder
+        # Replace USER placeholder and expand ${HOME}
         dir="${dir//USER/$USER}"
+        dir="${dir//\$\{HOME\}/$HOME}"
 
         if [ -d "$dir" ]; then
             check_pass "Log directory exists: $dir"

src/disk_manager.py

@@ -7,6 +7,7 @@
 import shutil
 import logging
 import time
+import fnmatch
 from pathlib import Path
 from typing import List, Tuple, Dict
 import os
@@ -53,41 +54,49 @@ class DiskManager:
         uploaded_files (dict): Files safe to delete with deletion time (NEW v2.0)
     """
 
-    def __init__(self, 
+    def __init__(self,
              log_directories: List[str],
              reserved_gb: float = 70.0,
              warning_threshold: float = 0.90,
-             critical_threshold: float = 0.95):
+             critical_threshold: float = 0.95,
+             directory_configs: Dict[str, Dict] = None):
         """
         Initialize disk manager.
-        
+
         Args:
             log_directories: Directories to monitor and clean
             reserved_gb: Minimum free space to maintain (GB)
             warning_threshold: Disk usage % to warn at (0-1, e.g. 0.90 = 90%)
             critical_threshold: Disk usage % to force cleanup (0-1, e.g. 0.95 = 95%)
-            
+            directory_configs: Optional dict mapping directory paths to their configs
+                              Format: {'/var/log': {'pattern': 'syslog.[1-9]*', 'recursive': False}}
+
         Note:
             Thresholds are disk usage percentages, not free space percentages
         """
         self.log_directories = [Path(d) for d in log_directories]
         self.reserved_bytes = int(reserved_gb * 1024 * 1024 * 1024)
         self.warning_threshold = warning_threshold
         self.critical_threshold = critical_threshold
-        
+
+        # Store directory configurations (pattern, recursive) for deletion filtering
+        self.directory_configs = directory_configs or {}
+
         # Track uploaded files with deletion time (NEW v2.0)
         # Format: {filepath: delete_after_timestamp}
         # If keep_days=0, timestamp is 0 (delete immediately)
         # If keep_days=14, timestamp is upload_time + 14 days
         self.uploaded_files: Dict[str, float] = {}
-        
+
         # Callback for registry cleanup (set by main.py)
         self._on_file_deleted_callback = None
-        
+
         logger.info("Initialized")
         logger.info(f"Reserved space: {reserved_gb} GB")
         logger.info(f"Warning threshold: {warning_threshold * 100}%")
         logger.info(f"Critical threshold: {critical_threshold * 100}%")
+        if self.directory_configs:
+            logger.info(f"Directory configs loaded for {len(self.directory_configs)} directories")
 
     def mark_uploaded(self, filepath: str, keep_until_days: int = 0):
         """
@@ -115,7 +124,46 @@ def mark_uploaded(self, filepath: str, keep_until_days: int = 0):
                 delete_after = time.time() + (keep_until_days * SECONDS_PER_DAY)
 
         self.uploaded_files[abs_path] = delete_after
-    
+
+    def _matches_pattern(self, file_path: Path) -> bool:
+        """
+        Check if file matches the configured pattern for its parent directory.
+
+        Args:
+            file_path: Path to the file to check
+
+        Returns:
+            bool: True if file matches pattern (or no pattern configured), False otherwise
+        """
+        # Find which monitored directory this file belongs to
+        for log_dir in self.log_directories:
+            try:
+                # Check if file is under this log directory
+                file_path.relative_to(log_dir)
+
+                # Found the parent directory, check pattern
+                dir_str = str(log_dir.resolve())
+                config = self.directory_configs.get(dir_str, {})
+                pattern = config.get('pattern')
+
+                if pattern is None:
+                    # No pattern configured - accept all files
+                    logger.debug(f"Deletion pattern check: {file_path.name} - no pattern, accepting")
+                    return True
+                else:
+                    # Check if filename matches pattern
+                    match_result = fnmatch.fnmatch(file_path.name, pattern)
+                    logger.debug(f"Deletion pattern check: {file_path.name} vs '{pattern}' => {match_result}")
+                    return match_result
+
+            except ValueError:
+                # file_path is not relative to this log_dir, continue
+                continue
+
+        # File is not in any monitored directory - should not happen, but be safe
+        logger.warning(f"File {file_path} not in any monitored directory, skipping deletion")
+        return False
+
     def get_disk_usage(self, path: str = "/") -> Tuple[float, int, int]:
         """Get disk usage statistics (returns: usage_percent, used_bytes, free_bytes)."""
         stat = shutil.disk_usage(path)
@@ -227,16 +275,21 @@ def cleanup_by_age(self, max_age_days: int) -> int:
 
             for file_path in directory.rglob('*'):
                 if file_path.is_file() and not file_path.name.startswith('.'):
+                    # NEW: Check if file matches the upload pattern before deletion
+                    if not self._matches_pattern(file_path):
+                        logger.debug(f"Skipping {file_path.name} - doesn't match upload pattern")
+                        continue
+
                     try:
                         mtime = file_path.stat().st_mtime
-                        
+
                         if mtime < cutoff_time:
                             size = file_path.stat().st_size
                             age_days = (time.time() - mtime) / 86400
-                            
+
                             logger.info(f"Deleting old file: {file_path.name} "
                                     f"({age_days:.1f} days old, {size / (1024**2):.1f} MB)")
-                            
+
                             file_path.unlink()
                             deleted_count += 1
                             freed_bytes += size
@@ -338,6 +391,11 @@ def emergency_cleanup_all_files(self, target_free_gb: float = None) -> int:
 
             for file_path in directory.rglob('*'):
                 if file_path.is_file() and not file_path.name.startswith('.'):
+                    # NEW: Check if file matches the upload pattern before deletion
+                    if not self._matches_pattern(file_path):
+                        logger.debug(f"EMERGENCY: Skipping {file_path.name} - doesn't match upload pattern")
+                        continue
+
                     try:
                         mtime = file_path.stat().st_mtime
                         size = file_path.stat().st_size

src/main.py

@@ -93,11 +93,22 @@ def __init__(self, config_path: str):
             log_directories=log_dir_configs
         )
 
+        # Build directory_configs for disk manager (pattern-aware deletion)
+        directory_configs = {}
+        if is_new_format:
+            for item in log_dir_configs:
+                resolved_path = str(Path(item['path']).expanduser().resolve())
+                directory_configs[resolved_path] = {
+                    'pattern': item.get('pattern'),
+                    'recursive': item.get('recursive', True)
+                }
+
         self.disk_manager = DiskManager(
             log_directories=monitor_paths,
             reserved_gb=self.config.get('disk.reserved_gb'),
             warning_threshold=self.config.get('disk.warning_threshold', 0.90),
-            critical_threshold=self.config.get('disk.critical_threshold', 0.95)
+            critical_threshold=self.config.get('disk.critical_threshold', 0.95),
+            directory_configs=directory_configs if is_new_format else None
         )
 
         def on_file_deleted(filepath):

tests/unit/test_config.py

@@ -644,7 +644,7 @@ def test_recursive_boolean_false():
 
 
 def test_recursive_string_value_rejected():
-    """Test recursive: "true" (string) is accepted (YAML converts to bool)"""
+    """Test recursive: "true" (string) is rejected by type validation"""
     config = {
         'vehicle_id': 'vehicle-001',
         'log_directories': [
@@ -660,14 +660,15 @@ def test_recursive_string_value_rejected():
         temp_path = f.name
 
     try:
-        # YAML will keep the string "true", but validation doesn't check type
-        cm = ConfigManager(temp_path)
+        # Validation now checks type - string should be rejected
+        with pytest.raises(ConfigValidationError, match="recursive.*boolean"):
+            cm = ConfigManager(temp_path)
     finally:
         Path(temp_path).unlink()
 
 
 def test_recursive_integer_value_rejected():
-    """Test recursive: 1 (integer) is accepted (validation doesn't check type)"""
+    """Test recursive: 1 (integer) is rejected by type validation"""
     config = {
         'vehicle_id': 'vehicle-001',
         'log_directories': [
@@ -683,8 +684,9 @@ def test_recursive_integer_value_rejected():
         temp_path = f.name
 
     try:
-        # Validation doesn't check type for recursive field
-        cm = ConfigManager(temp_path)
+        # Validation now checks type - integer should be rejected
+        with pytest.raises(ConfigValidationError, match="recursive.*boolean"):
+            cm = ConfigManager(temp_path)
     finally:
         Path(temp_path).unlink()
 
@@ -743,7 +745,7 @@ def test_pattern_empty_string_rejected():
 
 
 def test_pattern_non_string_rejected():
-    """Test non-string pattern is accepted (validation doesn't check type)"""
+    """Test non-string pattern is rejected by type validation"""
     config = {
         'vehicle_id': 'vehicle-001',
         'log_directories': [
@@ -759,8 +761,9 @@ def test_pattern_non_string_rejected():
         temp_path = f.name
 
     try:
-        # Validation doesn't check type for pattern field
-        cm = ConfigManager(temp_path)
+        # Validation now checks type - integer should be rejected
+        with pytest.raises(ConfigValidationError, match="pattern.*string"):
+            cm = ConfigManager(temp_path)
     finally:
         Path(temp_path).unlink()