Ревью и рефакторинг PowerShell-скрипта

Ревью скрипта очистки бэкапов SQL

Что скрипт делает на самом деле

По замыслу скрипт должен удалять старые бэкапы и сообщать о проблемах. Фактически он работает так:

1. Пытается перечислить все файлы (не только бэкапы) под $BackupRoot. Любые ошибки перечисления он глушит. Если шара недоступна, получает $null и об этом не узнаёт.
2. Удаляет любые файлы с LastWriteTime старше 14 дней, без учёта того, сколько бэкапов останется. Ошибки удаления тоже глушит.
3. Про каждый файл-кандидат пишет в лог «Удалён», даже если файл удалить не удалось.
4. Проверка свободного места для UNC-пути не работает вообще. "\\FS01\..." даёт .Substring(0,1) = "\", диска с именем \ нет, Get-PSDrive выдаёт ошибку, $null / 1GB = 0, условие 0 -lt 10 истинно. Значит, каждую ночь либо уходит письмо «Мало места… Осталось 0 GB», либо Send-MailMessage падает. Что из этого происходит у вас, по коду не определить, проверьте ящик it-alerts. Если письмо приходит каждую ночь, его давно никто не читает. Это может объяснять, почему три дня никто ничего не замечал.
5. Всегда пишет «Очистка завершена», выводит OK и завершается с кодом 0. Планировщик показывает 0x0.

Главная находка. Скрипт не может сообщить о сбое ни одним из своих каналов: лог врёт, код возврата всегда 0, письмо о месте ложное. Оба ваших инцидента прямо следуют из кода, это не случайности.

Находки по убыванию важности

🔴 1. Потеря данных: нет защиты от удаления всех бэкапов и нет пробного режима

Несколько сценариев, при которых скрипт уничтожит последние рабочие бэкапы:

• Бэкапы перестали создаваться (упал SQL Agent, сменился пароль учётки, кончилось место). Через 14 дней скрипт удалит последнюю копию. Удаление по одному только возрасту без правила «оставить минимум N последних» — главный риск этого скрипта.
• $RetentionDays без валидации. При запуске с -RetentionDays 0 или отрицательным значением $cutoff оказывается в настоящем или будущем, и удаляется всё.
• $BackupRoot произвольный. Если кто-то запустит скрипт с -BackupRoot \\FS01\Backups или D:\, он рекурсивно удалит всё старше 14 дней в этом месте.
• Удаляются любые файлы, а не только .bak/.trn. Рядом с бэкапами часто лежат бэкапы сертификатов TDE и мастер-ключей. Они старые по определению и без них зашифрованные бэкапы не восстановить.
• Нет -WhatIf. Проверить поведение перед выкаткой невозможно, только удалить по-настоящему.

Исправление приведено в полной версии ниже:

• ValidateRange на параметры;
• белый список корней;
• фильтр по расширениям;
• правило «оставить N последних в каждой папке»;
• аварийный стоп, если к удалению намечено подозрительно большое число файлов;
• SupportsShouldProcess для -WhatIf.

🔴 2. Потеря данных: Remove-Item $f.FullName интерпретирует [ ] как шаблон

Позиционный аргумент попадает в -Path, а -Path поддерживает подстановочные знаки. Возьмём старый файл DB[12].bak. Для -Path это шаблон, которому соответствуют DB1.bak и DB2.bak. Они могут оказаться свежими файлами, и тогда удалятся именно они. Сам DB[12].bak при этом не найдётся, ошибка будет подавлена, а в лог попадёт «Удалён».

Для типичных имён бэкапов вероятность низкая, но цена ошибки высокая, а исправление тривиальное:

Remove-Item -LiteralPath $f.FullName -Force -ErrorAction Stop

То же касается Get-ChildItem: там тоже нужен -LiteralPath.

🟠 3. Тихий неверный результат: ошибка перечисления подавлена, catch никогда не срабатывает (ваш инцидент №1)

try {
    $files = Get-ChildItem ... -ErrorAction SilentlyContinue
} catch { ... }

try/catch ловит только прерывающие ошибки. SilentlyContinue делает прямо противоположное: превращает ошибку в незаметную. catch здесь мёртвый код. Но даже если бы он сработал, скрипт всё равно пошёл бы дальше.

Дальше в PowerShell 5.1:

• $files = $null;
• $oldFiles = $null;
• foreach по $null не выполняется ни разу;
• $null.Count = 0.

Итог: «Удалено файлов: 0» и OK. Ровно это вы и видели.

Вторая, менее заметная проблема: если недоступна одна подпапка (права, путь длиннее 260 символов), она молча пропускается. Файлы в ней не удаляются никогда, и диск постепенно заполняется.

Исправление:

if (-not (Test-Path -LiteralPath $BackupRoot -PathType Container)) {
    throw "Путь недоступен: $BackupRoot"
}
$enumErrors = @()
$all = @(Get-ChildItem -LiteralPath $BackupRoot -Recurse -File `
         -ErrorAction SilentlyContinue -ErrorVariable enumErrors)
foreach ($e in $enumErrors) { Write-Log "Ошибка чтения: $($e.TargetObject) — $($e.Exception.Message)" }

$backups = @($all | Where-Object { $Extensions -contains $_.Extension })
if ($backups.Count -eq 0) { throw "Не найдено ни одного бэкапа в $BackupRoot — шара пуста или не прочиталась" }

Здесь SilentlyContinue остаётся, но ошибки собираются в -ErrorVariable и записываются в лог. Так подавление перестаёт быть слепым. Отдельно добавлена проверка «ноль бэкапов = авария». На живой шаре бэкапов ноль не бывает, поэтому пустой результат почти наверняка означает сбой, а не норму.

🟠 4. Тихий неверный результат: «Удалён» пишется безусловно, счётчик считает кандидатов (ваш инцидент №2)

Remove-Item $f.FullName -Force -ErrorAction SilentlyContinue
"Удалён: $($f.FullName)" | Out-File $logFile -Append

Результат удаления никак не проверяется. -Force снимает атрибут «только чтение», но не блокировку файла другим процессом. Итоговое «Удалено файлов: $($oldFiles.Count)» — это число кандидатов, а не удалённых файлов.

Исправление:

try {
    Remove-Item -LiteralPath $f.FullName -Force -ErrorAction Stop
    $deleted++
    Write-Log "Удалён: $($f.FullName)"
} catch {
    $failed++
    Write-Log "НЕ удалён: $($f.FullName) — $($_.Exception.Message)"
}

Повторять попытку внутри одного запуска не нужно. Файл, занятый агентом бэкапа, спокойно удалится следующей ночью, важно только, чтобы лог говорил правду.

🟠 5. Тихий неверный результат: проверка свободного места не проверяет шару

Механизм описан выше. Get-PSDrive и [IO.DriveInfo] не работают с UNC-путями. Надёжный вариант — GetDiskFreeSpaceEx: функция принимает UNC напрямую и не зависит от того, на какой машине выполняется скрипт.

function Get-UncFreeGB([string]$Path) {
    if (-not ('Win32.DiskSpace' -as [type])) {
        Add-Type -Namespace Win32 -Name DiskSpace -MemberDefinition @'
[DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)]
[return: MarshalAs(UnmanagedType.Bool)]
public static extern bool GetDiskFreeSpaceEx(string lpDirectoryName,
    out ulong lpFreeBytesAvailable, out ulong lpTotalNumberOfBytes, out ulong lpTotalNumberOfFreeBytes);
'@
    }
    $free = [uint64]0; $total = [uint64]0; $totalFree = [uint64]0
    if (-not [Win32.DiskSpace]::GetDiskFreeSpaceEx($Path.TrimEnd('\') + '\', [ref]$free, [ref]$total, [ref]$totalFree)) {
        throw "GetDiskFreeSpaceEx($Path): код ошибки $([Runtime.InteropServices.Marshal]::GetLastWin32Error())"
    }
    [math]::Round($free / 1GB, 1)
}

После исправления, скорее всего, перестанет приходить ежедневное ложное письмо, если оно у вас приходит. Предупредите коллег, что теперь письмо о месте означает реальную проблему.

🟠 6. Код возврата всегда 0

В каком бы состоянии ни завершился скрипт, планировщик видит 0x0. Нужен exit с ненулевым кодом при сбое. Чтобы код дошёл до планировщика, действие задачи должно выглядеть так:

powershell.exe -NoProfile -NonInteractive -ExecutionPolicy Bypass -File "C:\Scripts\cleanup-backups.ps1"

Используйте именно -File. С -Command код возврата теряется.

Сам по себе ненулевой код никого не оповестит. Поэтому при сбое в исправленной версии уходит письмо. Код возврата нужен для истории задач и для мониторинга, если он у вас смотрит в планировщик.

🟡 7. Неудобства и мелкие отказы

• Send-MailMessage без try. Если SMTP недоступен, скрипт может оборваться, не дописав итоговую строку лога. Отправку нужно обернуть в try/catch.
• Кодировка письма. В 5.1 кириллица в теме и теле без -Encoding UTF8 часто приходит вопросительными знаками. Добавьте -Encoding ([Text.Encoding]::UTF8).
• Папка лога. Если C:\Scripts\Logs не существует, Out-File падает, и в запуске из планировщика этого никто не увидит. Лечится одной строкой: New-Item -ItemType Directory -Force.
• Нет меток времени. У строк лога нет времени, есть только заголовок запуска. При разборе инцидентов это неудобно.
• Лог растёт бесконечно. Это не срочно: примерно сотня строк за ночь — годы до проблемы. Но ротацию стоит завести, когда до неё дойдут руки.
• Get-ChildItem -File без -Force пропускает скрытые файлы. Для бэкапов SQL это обычно не важно, упоминаю для полноты.

🟡 8. Проверьте вручную, а не в коде: цепочки восстановления

Удаление по возрасту отдельного файла не учитывает, что .trn и .dif бесполезны без своего полного бэкапа. Пример: полный бэкап делается раз в неделю, хранение 14 дней. Тогда журналы за дни 8–14 назад могут ссылаться на полный бэкап 15-дневной давности, который уже удалён. Восстановление на недавние точки при этом работает, а «на любую точку за 14 дней» — нет.

Если полные бэкапы делаются реже раза в неделю, проблема серьёзнее. Код скрипта это не исправит. Сверьте фактическое окно восстановления с тем, что обещано.

Отдельно об обработке ошибок

В скрипте три SilentlyContinue и ни одной проверки после них. Каждое из этих мест давало ложный успех:

Общее правило для исправленной версии: $ErrorActionPreference = 'Stop'. Там, где ошибку допустимо пропустить (перечисление), используется SilentlyContinue вместе с -ErrorVariable и записью в лог. Удаление каждого файла идёт в отдельном try/catch.

Крайние случаи

По длинным путям: не буду утверждать, что префикс \\?\UNC\ надёжно работает с Get-ChildItem в 5.1 на вашей сборке. Исправленная версия как минимум делает проблему видимой. Если в логе появятся PathTooLongException, это отдельная задача.

Идемпотентность и повторный запуск

С этим всё в порядке, менять не нужно. Удаление по возрасту идемпотентно по своей природе. Если запуск оборвался посередине, следующий просто доудалит оставшееся, а уже удалённые файлы в выборку не попадут. После трёх дней простоя следующий запуск удалит накопившиеся за это время файлы. Это около 3/17 от общего числа, аварийный стоп на это не сработает.

Одновременные запуски: проверьте, что в задаче стоит «Если задача уже выполняется: не запускать новый экземпляр» (это значение по умолчанию). Блокировку в коде городить не нужно.

Исправленная версия

Допущение: бэкапы разложены по папкам вида «сервер\база\тип», как в скриптах Ola Hallengren. Тогда «N последних в каждой папке» означает «N последних бэкапов каждого типа для каждой базы». Если все файлы лежат в одной папке, правило защиты нужно группировать по имени базы из имени файла. Пришлите пример структуры, и я перепишу этот фрагмент.

[CmdletBinding(SupportsShouldProcess = $true)]
param(
    [ValidateNotNullOrEmpty()]
    [string]$BackupRoot = '\\FS01\Backups\SQL',

    [ValidateRange(3, 365)]
    [int]$RetentionDays = 14,

    [ValidateRange(1, 100)]
    [int]$KeepLatestPerFolder = 3,     # никогда не удалять N последних бэкапов в каждой папке

    [ValidateRange(1, 100)]
    [int]$MaxDeletePercent = 40,       # стоп, если за один запуск удаляется больше этой доли

    [string[]]$Extensions = @('.bak', '.trn', '.dif'),

    [int]$MinFreeGB = 10
)

$ErrorActionPreference = 'Stop'
$allowedRoots = @('\\FS01\Backups\SQL')
$logDir  = 'C:\Scripts\Logs'
$logFile = Join-Path $logDir 'cleanup-backups.log'
$mail = @{
    To         = 'it-alerts@contoso.local'
    From       = 'backup-bot@contoso.local'
    SmtpServer = 'smtp.contoso.local'
    Encoding   = [System.Text.Encoding]::UTF8
}

function Write-Log([string]$Message) {
    # -WhatIf:$false — иначе в пробном режиме Out-File тоже ничего не запишет
    "$(Get-Date -Format 'yyyy-MM-dd HH:mm:ss')  $Message" |
        Out-File -FilePath $logFile -Append -WhatIf:$false
}

function Send-Alert([string]$Subject, [string]$Body) {
    try   { Send-MailMessage @mail -Subject $Subject -Body $Body -ErrorAction Stop }
    catch { Write-Log "Не удалось отправить письмо '$Subject': $($_.Exception.Message)" }
}

function Get-UncFreeGB([string]$Path) {
    if (-not ('Win32.DiskSpace' -as [type])) {
        Add-Type -Namespace Win32 -Name DiskSpace -MemberDefinition @'
[DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)]
[return: MarshalAs(UnmanagedType.Bool)]
public static extern bool GetDiskFreeSpaceEx(string lpDirectoryName,
    out ulong lpFreeBytesAvailable, out ulong lpTotalNumberOfBytes, out ulong lpTotalNumberOfFreeBytes);
'@
    }
    $free = [uint64]0; $total = [uint64]0; $totalFree = [uint64]0
    if (-not [Win32.DiskSpace]::GetDiskFreeSpaceEx($Path.TrimEnd('\') + '\', [ref]$free, [ref]$total, [ref]$totalFree)) {
        throw "GetDiskFreeSpaceEx($Path): код ошибки $([Runtime.InteropServices.Marshal]::GetLastWin32Error())"
    }
    [math]::Round($free / 1GB, 1)
}

New-Item -ItemType Directory -Path $logDir -Force -WhatIf:$false | Out-Null
$mode = if ($WhatIfPreference) { 'ПРОБНЫЙ ПРОГОН' } else { 'боевой режим' }
Write-Log "=== Запуск ($mode): $BackupRoot, хранение $RetentionDays дн., защита $KeepLatestPerFolder посл. в папке ==="

$exitCode = 0
try {
    # --- Защита от неверного пути ---
    if ($allowedRoots -notcontains $BackupRoot.TrimEnd('\')) {
        throw "Путь $BackupRoot не входит в список разрешённых"
    }
    if (-not (Test-Path -LiteralPath $BackupRoot -PathType Container)) {
        throw "Путь недоступен: $BackupRoot"
    }

    # --- Перечисление: ошибки не глушим, а собираем ---
    $cutoff = (Get-Date).AddDays(-$RetentionDays)
    $enumErrors = @()
    $all = @(Get-ChildItem -LiteralPath $BackupRoot -Recurse -File `
             -ErrorAction SilentlyContinue -ErrorVariable enumErrors)
    foreach ($e in $enumErrors) {
        Write-Log "Ошибка чтения: $($e.TargetObject) — $($e.Exception.Message)"
    }

    $backups = @($all | Where-Object { $Extensions -contains $_.Extension })
    if ($backups.Count -eq 0) {
        throw "Не найдено ни одного бэкапа в $BackupRoot — шара пуста или не прочиталась"
    }

    # --- Защита: N последних в каждой папке не трогаем никогда ---
    $protected = @{}
    $backups | Group-Object DirectoryName | ForEach-Object {
        $_.Group | Sort-Object LastWriteTime -Descending |
            Select-Object -First $KeepLatestPerFolder |
            ForEach-Object { $protected[$_.FullName] = $true }
    }

    $toDelete = @($backups | Where-Object {
        $_.LastWriteTime -lt $cutoff -and -not $protected.ContainsKey($_.FullName)
    })

    # --- Аварийный стоп на подозрительно большое удаление ---
    $pct = [math]::Round([double]$toDelete.Count / $backups.Count * 100, 1)
    if ($pct -gt $MaxDeletePercent) {
        throw "К удалению $($toDelete.Count) из $($backups.Count) ($pct%) — больше порога $MaxDeletePercent%. Ничего не удалено, проверьте вручную."
    }

    # --- Удаление с проверкой результата ---
    $deleted = 0; $failed = 0
    foreach ($f in $toDelete) {
        if ($PSCmdlet.ShouldProcess($f.FullName, 'Удалить')) {
            try {
                Remove-Item -LiteralPath $f.FullName -Force -ErrorAction Stop
                $deleted++
                Write-Log "Удалён: $($f.FullName)"
            }
            catch {
                $failed++
                Write-Log "НЕ удалён: $($f.FullName) — $($_.Exception.Message)"
            }
        }
        elseif ($WhatIfPreference) {
            Write-Log "[WhatIf] был бы удалён: $($f.FullName) ($($f.LastWriteTime))"
        }
    }

    Write-Log "Итог: бэкапов найдено $($backups.Count), к удалению $($toDelete.Count), удалено $deleted, ошибок удаления $failed, ошибок чтения $($enumErrors.Count)"

    # --- Место на шаре: отдельный try, чтобы сбой проверки не выглядел как сбой очистки ---
    try {
        $freeGB = Get-UncFreeGB $BackupRoot
        Write-Log "Свободно на шаре: $freeGB GB"
        if ($freeGB -lt $MinFreeGB) {
            Send-Alert "Мало места на $BackupRoot" "Осталось $freeGB GB (порог $MinFreeGB GB)."
        }
    }
    catch {
        $exitCode = 1
        Write-Log "Не удалось проверить свободное место: $($_.Exception.Message)"
        Send-Alert "Очистка бэкапов: не удалось проверить место" $_.Exception.Message
    }

    if ($enumErrors.Count -gt 0 -or $failed -gt 0) {
        $exitCode = 1
        Send-Alert "Очистка бэкапов: частичный сбой" `
            "Ошибок чтения: $($enumErrors.Count), не удалено файлов: $failed.`nЛог: $logFile на $env:COMPUTERNAME"
    }
}
catch {
    $exitCode = 2
    Write-Log "ОШИБКА, очистка прервана: $($_.Exception.Message)"
    Send-Alert "Очистка бэкапов НЕ выполнена" "$($_.Exception.Message)`nЛог: $logFile на $env:COMPUTERNAME"
}

Write-Log "=== Завершено, код $exitCode ==="
exit $exitCode

Пояснения к неочевидным решениям:

• -WhatIf:$false у Out-File и New-Item. В пробном режиме $WhatIfPreference наследуется всеми командлетами. Без этой отметки лог в пробном режиме не писался бы, и смысл прогона пропал бы.
• Отдельная ветка для [WhatIf]. Стандартное сообщение «What if:» уходит в консоль, а при запуске из планировщика консоли нет. Поэтому кандидаты на удаление явно пишутся в лог.
• Разные коды возврата. Частичный сбой (занятый файл, недоступная подпапка) даёт код 1 и письмо, но очистка продолжается. Полный сбой (шара недоступна, пусто, стоп по порогу) даёт код 2, и ничего не удаляется.
• Возможный шум от занятых файлов. Если агент бэкапа читает старые файлы каждую ночь, письма о частичном сбое станут регулярными. Тогда стоит слать письмо только при повторной неудаче одного и того же файла. Пока рекомендую посмотреть, как часто это реально происходит.
• Значения порогов. KeepLatestPerFolder = 3 и MaxDeletePercent = 40 взяты с запасом. Подберите их по результатам пробного прогона.

Что оставить как есть

• LastWriteTime, а не CreationTime. Выбор правильный: при копировании файла CreationTime сбрасывается на момент копирования, а LastWriteTime сохраняется.
• -Force у Remove-Item. Нужен, чтобы удалялись файлы с атрибутом «только чтение».
• Out-File с кодировкой по умолчанию. В 5.1 это UTF-16, и кириллица пишется корректно. Если сменить кодировку посреди существующего файла, получится файл в смешанной кодировке. Если очень хочется UTF-8, делайте это вместе с ротацией лога.
• Send-MailMessage. Командлет помечен как устаревший, но в 5.1 работает. Переход на другую библиотеку ради этого — вкусовщина.
• Рекурсия и сама схема «удалить старше N дней». Схема разумная, ей не хватало только защит.
• Блокировка от параллельных запусков. Не нужна, этим занимается планировщик.

Порядок работ

1. Сегодня, ничего не выкатывая. Проверьте ящик it-alerts: приходит ли каждую ночь «Мало места… 0 GB». Посмотрите, лежат ли на шаре файлы кроме .bak/.trn/.dif, особенно сертификаты (.cer, .pvk, .key). Если лежат, исходный скрипт уже мог удалить старые копии. Проверьте, что они есть где-то ещё.
2. Сверьте цепочки бэкапов с окном хранения (пункт 8) и уточните структуру папок. От неё зависит логика KeepLatestPerFolder.
3. Пробный прогон исправленной версии вручную от имени svc-backupcleanup с -WhatIf. Изучите в логе список кандидатов, процент удаляемого и свободное место.
4. Негативные тесты, тоже с -WhatIf. Недоступный путь (например, \\FS01\NoSuchShare, добавленный во временный белый список), -RetentionDays 0, пустая тестовая папка. Каждый случай должен давать письмо и код 2.
5. Выкатка. В задаче установите -File, -NoProfile, -NonInteractive. После первой боевой ночи проверьте лог и «Результат последнего запуска».
6. Потом, по желанию: ротация лога и уведомление только о повторных неудачах удаления одного и того же файла.

Пункты 1–4 закрывают оба ваших инцидента и риск потери данных. Остальное — улучшения, и их можно отложить.