From 29910a3ede36c915102e52f64ea4963fc41f2267 Mon Sep 17 00:00:00 2001 From: simonyang08 Date: Fri, 4 Sep 2026 13:56:42 +0800 Subject: [PATCH] Invoke-DbaDbLogShipping - avoid Azure blob name collision (#10667) When Invoke-DbaDbLogShipping backs up to Azure blob storage it builds the backup file name from a Get-Date timestamp with second resolution. Two CI runs that hit the cmdlet within the same wall-clock second produced the same FullBackup_PreLogShipping blob name on the shared Azure container, so the second run failed with 'Cannot open backup device ... Operating system error 50' even when nothing in the PR was actually wrong. Two minimal changes close the bug: * Production: build the Azure timestamp with millisecond precision (yyyyMMddHHmmssfff). Two callers that enter the cmdlet inside the same second now produce different blob names, regardless of how the caller picks the database name. * CI: scope the dbatoolsci_logship_azure database name with GITHUB_RUN_ID and GITHUB_RUN_ATTEMPT in .github/scripts/gh-actions.ps1, so concurrent linux-tests jobs (and reruns) never reuse the same database or blob. The Azure integration test still needs a real SQL Server plus the shared Azure container to actually run, so the regression is asserted on the shape of the inputs that drive the blob name: a new unit test reads the production source and the CI script and verifies the timestamp carries the millisecond 'fff' format specifier and the database name carries GITHUB_RUN_ID. This catches a future revert to second resolution or a revert to the hard-coded database name without requiring the Azure fixture. Signed-off-by: simonyang08 --- .github/scripts/gh-actions.ps1 | 11 ++- public/Invoke-DbaDbLogShipping.ps1 | 2 +- tests/Invoke-DbaDbLogShipping.Azure.Tests.ps1 | 82 +++++++++++++++++++ 3 files changed, 92 insertions(+), 3 deletions(-) create mode 100644 tests/Invoke-DbaDbLogShipping.Azure.Tests.ps1 diff --git a/.github/scripts/gh-actions.ps1 b/.github/scripts/gh-actions.ps1 index 66c51065376..8ac989a6122 100644 --- a/.github/scripts/gh-actions.ps1 +++ b/.github/scripts/gh-actions.ps1 @@ -353,7 +353,11 @@ END $cred = New-Object -TypeName System.Management.Automation.PSCredential -ArgumentList "sqladmin", $password $azureUrl = "https://dbatools.blob.core.windows.net/dbatools" - $dbName = "dbatoolsci_logship_azure" + # GITHUB_RUN_ID uniquely identifies this CI job; appending it to the database name keeps + # concurrent log-shipping tests from picking the same Azure blob name on the shared + # container. GITHUB_RUN_ATTEMPT is appended so a rerun still gets its own database. + # See https://github.com/dataplat/dbatools/issues/10667. + $dbName = "dbatoolsci_logship_azure_$($env:GITHUB_RUN_ID)_$($env:GITHUB_RUN_ATTEMPT)" # Create SAS token credential on both instances $primaryServer = Connect-DbaInstance -SqlInstance localhost -SqlCredential $cred @@ -507,7 +511,10 @@ END It -Skip:(-not $env:azurepasswd) "adds a second live secondary without replacing the Azure primary configuration" { $PSDefaultParameterValues.Clear() $azureUrl = "https://dbatools.blob.core.windows.net/dbatools" - $dbName = "dbatoolsci_logship_addsecondary" + # GITHUB_RUN_ID uniquely identifies this CI job; the addsecondary test + # uploads to the same shared Azure container, so its database name also + # needs the per-run suffix. See https://github.com/dataplat/dbatools/issues/10667. + $dbName = "dbatoolsci_logship_addsecondary_$($env:GITHUB_RUN_ID)_$($env:GITHUB_RUN_ATTEMPT)" $secondDbName = "${dbName}_second" $missingPrimaryDbName = "${dbName}_missing" $sasToken = $env:azurepasswd.TrimStart("?") diff --git a/public/Invoke-DbaDbLogShipping.ps1 b/public/Invoke-DbaDbLogShipping.ps1 index 05e4a1b5b14..3c28b43d2ea 100644 --- a/public/Invoke-DbaDbLogShipping.ps1 +++ b/public/Invoke-DbaDbLogShipping.ps1 @@ -1660,7 +1660,7 @@ WHERE pd.primary_database = N'$escapedPrimaryDatabase' Write-Message -Message "Backing up database $db to $DatabaseSharedPath" -Level Verbose try { - $Timestamp = Get-Date -format "yyyyMMddHHmmss" + $Timestamp = Get-Date -Format "yyyyMMddHHmmssfff" if ($UseAzure) { # Backup to Azure blob storage - use container base URL only diff --git a/tests/Invoke-DbaDbLogShipping.Azure.Tests.ps1 b/tests/Invoke-DbaDbLogShipping.Azure.Tests.ps1 new file mode 100644 index 00000000000..1b38e5360a4 --- /dev/null +++ b/tests/Invoke-DbaDbLogShipping.Azure.Tests.ps1 @@ -0,0 +1,82 @@ +#Requires -Module @{ ModuleName = "Pester"; ModuleVersion = "5.0" } +<# + Regression coverage for the Azure log shipping blob name collision reported + in https://github.com/dataplat/dbatools/issues/10667. + + Two concurrent CI runs that reach Invoke-DbaDbLogShipping within the same + wall-clock second used to write the same FullBackup_PreLogShipping blob to + the shared Azure container, because: + + 1. The production timestamp was built with Get-Date -Format "yyyyMMddHHmmss" + (second resolution). + 2. The CI test pinned the database names ("dbatoolsci_logship_azure" and + "dbatoolsci_logship_addsecondary"), so the per-second collision was not + bounded to a single runner. + + The Azure test path needs a real SQL Server plus a real Azure container, so + the actual collision cannot be reproduced in a unit environment. The + regression is therefore asserted on the shape of the inputs that drive the + blob name: sub-second timestamp precision in the production cmdlet, and + per-run unique database names in the CI script. +#> +param( + $ModuleName = "dbatools", + $CommandName = "Invoke-DbaDbLogShipping", + $PSDefaultParameterValues = $TestConfig.Defaults +) + +Describe "$CommandName - Azure blob name collision (#10667)" -Tag UnitTests { + BeforeAll { + $RepoRoot = Resolve-Path (Join-Path $PSScriptRoot "..") + $ProductionFile = Join-Path $RepoRoot "public/Invoke-DbaDbLogShipping.ps1" + $CiTestScriptFile = Join-Path $RepoRoot ".github/scripts/gh-actions.ps1" + } + + Context "Production timestamp must include sub-second precision" { + It "uses a millisecond format specifier for the Azure pre-log-shipping timestamp" { + $ProductionFile | Should -Exist + + $productionContent = Get-Content -Path $ProductionFile -Raw + + # The offending line built the timestamp with second resolution only. + # It must contain the millisecond 'fff' format specifier so two + # callers that enter the cmdlet inside the same second no longer + # produce the same Azure blob name. + $timestampMatches = [regex]::Matches( + $productionContent, + 'Get-Date\s+(?:-Format|-format)\s+"(?[^"]+)"' + ) + $azureTimestampLine = $timestampMatches | Where-Object { + $PSItem.Groups['fmt'].Value -like '*yyyyMMddHHmmss*' -and + $PSItem.Groups['fmt'].Value -like '*fff*' + } | Select-Object -First 1 + + $timestampReason = "Invoke-DbaDbLogShipping must build the Azure pre-log-shipping timestamp with sub-second precision to avoid the collision in #10667" + $azureTimestampLine | Should -Not -BeNullOrEmpty -Because $timestampReason + } + } + + Context "CI test database names must be unique per run" { + It "scopes every log shipping database name with GITHUB_RUN_ID" { + $CiTestScriptFile | Should -Exist + + $ciScriptContent = Get-Content -Path $CiTestScriptFile -Raw + + # Every log shipping database name in the CI script must include a + # per-run token (GITHUB_RUN_ID, plus GITHUB_RUN_ATTEMPT for reruns) + # so two simultaneous CI jobs never collide on the shared Azure + # container. This covers both "dbatoolsci_logship_azure" and + # "dbatoolsci_logship_addsecondary". + $dbNameMatches = [regex]::Matches( + $ciScriptContent, + '\$dbName\s*=\s*"(?dbatoolsci_logship_[^"]+)"' + ) + $dbNameMatches | Should -Not -BeNullOrEmpty -Because "the Azure log shipping integration tests must declare per-run database names" + + foreach ($dbNameMatch in $dbNameMatches) { + $dbNameReason = "the database name used by the Azure log shipping tests must be unique per CI run (found: $($dbNameMatch.Groups['value'].Value))" + $dbNameMatch.Groups['value'].Value | Should -Match 'GITHUB_RUN_ID' -Because $dbNameReason + } + } + } +}