From 13efb1ce358addd538cd04313e00e193297180ad Mon Sep 17 00:00:00 2001 From: Rhys Bevilaqua Date: Thu, 29 Oct 2026 12:56:51 +0800 Subject: [PATCH 1/3] Add setup probe to make sure directory is writable for body storage --- .../FileSystemBodyStorageInstaller.cs | 20 ++++- .../EFCore/BodyStoragePersistenceTests.cs | 75 +++++++++++++++++++ 2 files changed, 92 insertions(+), 3 deletions(-) diff --git a/src/ServiceControl.Persistence.EFCore/Implementation/BodyStorage/FileSystemBodyStorageInstaller.cs b/src/ServiceControl.Persistence.EFCore/Implementation/BodyStorage/FileSystemBodyStorageInstaller.cs index 2766e08e3d..9c6bebceb1 100644 --- a/src/ServiceControl.Persistence.EFCore/Implementation/BodyStorage/FileSystemBodyStorageInstaller.cs +++ b/src/ServiceControl.Persistence.EFCore/Implementation/BodyStorage/FileSystemBodyStorageInstaller.cs @@ -5,10 +5,24 @@ namespace ServiceControl.Persistence.EFCore.Implementation.BodyStorage; public class FileSystemBodyStorageInstaller(FileSystemBodyStorageSettings settings) : IBodyStorageInstaller { - public Task Provision(CancellationToken cancellationToken = default) + public async Task Provision(CancellationToken cancellationToken = default) { - Directory.CreateDirectory(settings.StoragePath); + try + { + Directory.CreateDirectory(settings.StoragePath); - return Task.CompletedTask; + var probePath = Path.Combine(settings.StoragePath, $".write-probe-{Guid.NewGuid():N}"); + await using var probe = new FileStream(probePath, FileMode.CreateNew, FileAccess.Write, FileShare.None, 4096, FileOptions.DeleteOnClose); + await probe.WriteAsync(new byte[] { 1 }, cancellationToken).ConfigureAwait(false); + await probe.FlushAsync(cancellationToken).ConfigureAwait(false); + } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + { + throw; + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + throw new InvalidOperationException($"Message body storage path '{settings.StoragePath}' is not writable. Ensure the directory can be created and the service account can write to it.", ex); + } } } diff --git a/src/ServiceControl.Persistence.Tests/EFCore/BodyStoragePersistenceTests.cs b/src/ServiceControl.Persistence.Tests/EFCore/BodyStoragePersistenceTests.cs index 070056f6d1..c3f9db1b6d 100644 --- a/src/ServiceControl.Persistence.Tests/EFCore/BodyStoragePersistenceTests.cs +++ b/src/ServiceControl.Persistence.Tests/EFCore/BodyStoragePersistenceTests.cs @@ -2,6 +2,7 @@ namespace ServiceControl.Persistence.Tests; using System; using System.IO; +using System.Runtime.Versioning; using System.Text; using System.Threading.Tasks; using NUnit.Framework; @@ -179,6 +180,80 @@ public async Task Provisioning_creates_the_filesystem_storage_directory() await new FileSystemBodyStorageInstaller(new FileSystemBodyStorageSettings { StoragePath = path }).Provision(); Assert.That(Directory.Exists(path), Is.True); + Assert.That(Directory.GetFileSystemEntries(path), Is.Empty); + } + + [Test] + public async Task Provisioning_keeps_existing_bodies() + { + var existing = Path.Combine(tempDir, "existing"); + await File.WriteAllTextAsync(existing, "body"); + + await new FileSystemBodyStorageInstaller(new FileSystemBodyStorageSettings { StoragePath = tempDir }).Provision(); + + Assert.That(Directory.GetFileSystemEntries(tempDir), Is.EqualTo(new[] { existing })); + } + + [Test] + public async Task Provisioning_fails_with_the_path_when_it_is_a_file() + { + var path = Path.Combine(tempDir, "file"); + await File.WriteAllTextAsync(path, "x"); + + var ex = Assert.ThrowsAsync(() => new FileSystemBodyStorageInstaller(new FileSystemBodyStorageSettings { StoragePath = path }).Provision()); + + using (Assert.EnterMultipleScope()) + { + Assert.That(ex.Message, Does.Contain(path)); + Assert.That(ex.InnerException, Is.InstanceOf()); + } + } + + [Test] + [UnsupportedOSPlatform("windows")] + public void Provisioning_fails_with_the_path_when_the_directory_is_not_writable() + { + if (OperatingSystem.IsWindows()) + { + Assert.Ignore("UnixFileMode is not supported on Windows."); + } + + var path = Path.Combine(tempDir, "readonly"); + Directory.CreateDirectory(path); + File.SetUnixFileMode(path, UnixFileMode.UserRead | UnixFileMode.UserExecute); + + try + { + if (CanWriteTo(path)) + { + Assert.Ignore("The current account bypasses directory permissions."); + } + + var ex = Assert.ThrowsAsync(() => new FileSystemBodyStorageInstaller(new FileSystemBodyStorageSettings { StoragePath = path }).Provision()); + + using (Assert.EnterMultipleScope()) + { + Assert.That(ex.Message, Does.Contain(path)); + Assert.That(ex.InnerException, Is.InstanceOf()); + } + } + finally + { + File.SetUnixFileMode(path, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); + } + } + + static bool CanWriteTo(string path) + { + try + { + using var _ = new FileStream(Path.Combine(path, Path.GetRandomFileName()), FileMode.CreateNew, FileAccess.Write, FileShare.None, 1, FileOptions.DeleteOnClose); + return true; + } + catch (UnauthorizedAccessException) + { + return false; + } } static byte[] ReadAll(Stream stream) From 10cc1b606104e1b1d51ce1dd4ae0cb537c651187 Mon Sep 17 00:00:00 2001 From: Rhys Bevilaqua Date: Fri, 30 Oct 2026 11:58:23 +0800 Subject: [PATCH 2/3] Add startup validation of database migrations and body storage --- ...ndling-unavailable-runtime-dependencies.md | 9 ++ .../When_hosting_error_ingestion_only.cs | 1 + .../Abstractions/EFPersistenceBase.cs | 3 + .../PendingMigrationsStartupCheck.cs | 18 +++ .../EFCore/SchemaSetupTests.cs | 39 +++++ .../HostApplicationBuilderExtensions.cs | 1 + .../HostApplicationBuilderExtensions.cs | 2 + .../StartupCheckHostedService.cs | 25 ++++ .../StartupCheckHostedServiceTests.cs | 141 ++++++++++++++++++ .../StartupChecks/IStartupCheck.cs | 9 ++ .../Abstractions/BasePersistence.cs | 4 + .../FileSystemBodyStorageInstaller.cs | 20 +-- .../FileSystemBodyStorageStartupCheck.cs | 26 ++++ .../PendingMigrationsStartupCheck.cs | 18 +++ .../SchemaSetupTests.cs | 34 +++++ .../SchemaSetupTests.cs | 34 +++++ .../EFCore/BodyStoragePersistenceTests.cs | 37 ++++- .../FileSystemBodyStorageCustomCheckTests.cs | 28 +++- .../HostApplicationBuilderExtensions.cs | 1 + 19 files changed, 427 insertions(+), 23 deletions(-) create mode 100644 src/ServiceControl.Audit.Persistence.EFCore/Implementation/PendingMigrationsStartupCheck.cs create mode 100644 src/ServiceControl.Hosting/StartupCheckHostedService.cs create mode 100644 src/ServiceControl.Infrastructure.Tests/StartupChecks/StartupCheckHostedServiceTests.cs create mode 100644 src/ServiceControl.Infrastructure/StartupChecks/IStartupCheck.cs create mode 100644 src/ServiceControl.Persistence.EFCore/Implementation/BodyStorage/FileSystemBodyStorageStartupCheck.cs create mode 100644 src/ServiceControl.Persistence.EFCore/Implementation/PendingMigrationsStartupCheck.cs diff --git a/docs/handling-unavailable-runtime-dependencies.md b/docs/handling-unavailable-runtime-dependencies.md index 1d7a82b180..963176cd41 100644 --- a/docs/handling-unavailable-runtime-dependencies.md +++ b/docs/handling-unavailable-runtime-dependencies.md @@ -17,3 +17,12 @@ ServiceControl instances handle unavailable runtime dependencies in one of two w * A failure detected during startup stops the instance immediately. * A failure detected after a successful startup does not stop the instance. The instance is expected to try to recover once the dependency becomes available again. + +### Startup checks + +The Main and Audit instances run startup checks (`IStartupCheck`) in a hosted service that is registered before persistence, ingestion, and component hosted services. A failing check stops the instance before it ingests any message. The SQL Server and PostgreSQL persisters register these checks: + +| ClassName | Prerequisites | Description | +| --- | --- | --- | +| `PendingMigrationsStartupCheck` | Main or Audit instance using SQL Server or PostgreSQL persistence | Checks that the database has no pending migrations. Runtime startup does not apply migrations; run `--setup` to apply them. | +| `FileSystemBodyStorageStartupCheck` | Main instance using SQL Server or PostgreSQL persistence with file system body storage | Checks that the configured body storage directory exists and is writable. Setup creates the directory but does not verify write access. | diff --git a/src/ServiceControl.AcceptanceTests/Recoverability/When_hosting_error_ingestion_only.cs b/src/ServiceControl.AcceptanceTests/Recoverability/When_hosting_error_ingestion_only.cs index 092652e7d9..58cef8074f 100644 --- a/src/ServiceControl.AcceptanceTests/Recoverability/When_hosting_error_ingestion_only.cs +++ b/src/ServiceControl.AcceptanceTests/Recoverability/When_hosting_error_ingestion_only.cs @@ -92,6 +92,7 @@ public async Task Should_default_the_concurrency_to_10_when_none_is_configured() static readonly string[] ExpectedHostedServices = [ "GenericWebHostService", // health endpoint only, no ServiceControl API + "StartupCheckHostedService", nameof(ErrorIngestion), // the reason this host exists "HeartbeatMonitoringHostedService", // warms the endpoint monitor, does not check heartbeats "InternalCustomChecksHostedService", // reports this node's ingestion health to the database diff --git a/src/ServiceControl.Audit.Persistence.EFCore/Abstractions/EFPersistenceBase.cs b/src/ServiceControl.Audit.Persistence.EFCore/Abstractions/EFPersistenceBase.cs index 90cb94678b..12e1278752 100644 --- a/src/ServiceControl.Audit.Persistence.EFCore/Abstractions/EFPersistenceBase.cs +++ b/src/ServiceControl.Audit.Persistence.EFCore/Abstractions/EFPersistenceBase.cs @@ -5,6 +5,7 @@ namespace ServiceControl.Audit.Persistence.EFCore.Abstractions; using NServiceBus; using ServiceControl.Audit.Persistence.EFCore.Implementation; using ServiceControl.Audit.Persistence.EFCore.Implementation.UnitOfWork; +using ServiceControl.Infrastructure.StartupChecks; using ServiceControl.Audit.Persistence.UnitOfWork; abstract class EFPersistenceBase(EFPersisterSettings settings) : IPersistence @@ -14,6 +15,8 @@ public void AddPersistence(IServiceCollection services) AddCommon(services); AddQueryServices(services); + services.AddScoped(); + services.AddSingleton(); services.AddSingleton(provider => provider.GetRequiredService()); services.AddSingleton(provider => provider.GetRequiredService()); diff --git a/src/ServiceControl.Audit.Persistence.EFCore/Implementation/PendingMigrationsStartupCheck.cs b/src/ServiceControl.Audit.Persistence.EFCore/Implementation/PendingMigrationsStartupCheck.cs new file mode 100644 index 0000000000..1737256f08 --- /dev/null +++ b/src/ServiceControl.Audit.Persistence.EFCore/Implementation/PendingMigrationsStartupCheck.cs @@ -0,0 +1,18 @@ +namespace ServiceControl.Audit.Persistence.EFCore.Implementation; + +using Microsoft.EntityFrameworkCore; +using ServiceControl.Audit.Persistence.EFCore.DbContexts; +using ServiceControl.Infrastructure.StartupChecks; + +sealed class PendingMigrationsStartupCheck(AuditDbContext dbContext) : IStartupCheck +{ + public async Task Check(CancellationToken cancellationToken = default) + { + var pending = (await dbContext.Database.GetPendingMigrationsAsync(cancellationToken).ConfigureAwait(false)).ToList(); + + if (pending.Count > 0) + { + throw new InvalidOperationException($"The audit database has pending migrations: {string.Join(", ", pending)}. Run the instance with --setup to apply them."); + } + } +} diff --git a/src/ServiceControl.Audit.Persistence.Tests/EFCore/SchemaSetupTests.cs b/src/ServiceControl.Audit.Persistence.Tests/EFCore/SchemaSetupTests.cs index d758095561..f574b7b6b8 100644 --- a/src/ServiceControl.Audit.Persistence.Tests/EFCore/SchemaSetupTests.cs +++ b/src/ServiceControl.Audit.Persistence.Tests/EFCore/SchemaSetupTests.cs @@ -6,6 +6,8 @@ namespace ServiceControl.Audit.Persistence.Tests using Microsoft.Extensions.Hosting; using NUnit.Framework; using ServiceControl.Audit.Persistence.EFCore.Abstractions; + using ServiceControl.Audit.Persistence.EFCore.DbContexts; + using ServiceControl.Audit.Persistence.EFCore.Implementation; using ServiceControl.Persistence.Tests; [TestFixture] @@ -73,6 +75,43 @@ public async Task Setup_ignores_the_primary_migrations_history_table_in_another_ } } + [Test] + public async Task Pending_migrations_fail_the_startup_check_until_setup_has_applied_them() + { + var testsConfiguration = new PersistenceTestsConfiguration(); + var connectionString = await testsConfiguration.GetConnectionString(); + var schema = $"sc_pending_{Guid.NewGuid():n}"; + + try + { + var exception = Assert.ThrowsAsync(() => CheckPendingMigrations(testsConfiguration, connectionString, schema)); + Assert.That(exception.Message, Does.Contain("pending migrations").And.Contain("--setup")); + + await Setup(testsConfiguration, connectionString, schema); + + Assert.DoesNotThrowAsync(() => CheckPendingMigrations(testsConfiguration, connectionString, schema)); + } + finally + { + await TestSchema.Drop(connectionString, schema); + } + } + + static async Task CheckPendingMigrations(PersistenceTestsConfiguration testsConfiguration, string connectionString, string schema) + { + var settings = new PersistenceSettings(TimeSpan.FromDays(1), true, 100000); + settings.PersisterSpecificSettings[EFPersistenceConfigurationBase.ConnectionStringKey] = connectionString; + settings.PersisterSpecificSettings[EFPersistenceConfigurationBase.SchemaKey] = schema; + + var services = new ServiceCollection(); + testsConfiguration.CreateConfiguration().Create(settings).AddInstaller(services); + + await using var provider = services.BuildServiceProvider(); + using var scope = provider.CreateScope(); + + await new PendingMigrationsStartupCheck(scope.ServiceProvider.GetRequiredService()).Check(); + } + static async Task Setup(PersistenceTestsConfiguration testsConfiguration, string connectionString, string schema) { var settings = new PersistenceSettings(TimeSpan.FromDays(1), true, 100000); diff --git a/src/ServiceControl.Audit/HostApplicationBuilderExtensions.cs b/src/ServiceControl.Audit/HostApplicationBuilderExtensions.cs index b7925ee4db..be2856badd 100644 --- a/src/ServiceControl.Audit/HostApplicationBuilderExtensions.cs +++ b/src/ServiceControl.Audit/HostApplicationBuilderExtensions.cs @@ -69,6 +69,7 @@ public static void AddServiceControlAudit(this IHostApplicationBuilder builder, // directly and to make things more complex of course the order of registration still matters ;) services.AddSingleton(provider => new Lazy(provider.GetRequiredService)); + services.AddStartupChecks(); services.AddPersistence(persistenceSettings, persistenceConfiguration); NServiceBusFactory.Configure(settings, transportCustomization, transportSettings, onCriticalError, configuration); diff --git a/src/ServiceControl.Hosting/HostApplicationBuilderExtensions.cs b/src/ServiceControl.Hosting/HostApplicationBuilderExtensions.cs index b221b73663..d30162da4f 100644 --- a/src/ServiceControl.Hosting/HostApplicationBuilderExtensions.cs +++ b/src/ServiceControl.Hosting/HostApplicationBuilderExtensions.cs @@ -14,4 +14,6 @@ public static void AddWindowsServiceWithRequestTimeout(this IHostApplicationBuil builder.Services.AddSingleton(); } } + + public static void AddStartupChecks(this IServiceCollection services) => services.AddHostedService(); } \ No newline at end of file diff --git a/src/ServiceControl.Hosting/StartupCheckHostedService.cs b/src/ServiceControl.Hosting/StartupCheckHostedService.cs new file mode 100644 index 0000000000..195568555a --- /dev/null +++ b/src/ServiceControl.Hosting/StartupCheckHostedService.cs @@ -0,0 +1,25 @@ +namespace ServiceControl.Hosting; + +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using ServiceControl.Infrastructure.StartupChecks; + +sealed class StartupCheckHostedService(IServiceScopeFactory scopeFactory) : IHostedService +{ + public async Task StartAsync(CancellationToken cancellationToken = default) + { + var scope = scopeFactory.CreateAsyncScope(); + + await using (scope.ConfigureAwait(false)) + { + foreach (var check in scope.ServiceProvider.GetServices()) + { + await check.Check(cancellationToken).ConfigureAwait(false); + } + } + } + + public Task StopAsync(CancellationToken cancellationToken = default) => Task.CompletedTask; +} diff --git a/src/ServiceControl.Infrastructure.Tests/StartupChecks/StartupCheckHostedServiceTests.cs b/src/ServiceControl.Infrastructure.Tests/StartupChecks/StartupCheckHostedServiceTests.cs new file mode 100644 index 0000000000..781e653190 --- /dev/null +++ b/src/ServiceControl.Infrastructure.Tests/StartupChecks/StartupCheckHostedServiceTests.cs @@ -0,0 +1,141 @@ +namespace ServiceControl.Infrastructure.Tests.StartupChecks; + +using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using NUnit.Framework; +using ServiceControl.Hosting; +using ServiceControl.Infrastructure.StartupChecks; + +[TestFixture] +class StartupCheckHostedServiceTests +{ + List events; + + [SetUp] + public void SetUp() => events = []; + + [Test] + public async Task Checks_run_in_order_in_one_scope_that_is_disposed_before_later_hosted_services() + { + using var host = Build(services => + { + services.AddSingleton(events); + services.AddScoped(); + services.AddScoped(p => Resolve(p, "first")); + services.AddScoped(p => Resolve(p, "second")); + }); + + await host.StartAsync(); + + Assert.That(events, Is.EqualTo(new[] { "scope created", "first", "second", "scope disposed", "later service started" })); + } + + [Test] + public void A_failing_check_stops_startup_before_later_hosted_services() + { + using var host = Build(services => + { + services.AddScoped(_ => new ThrowingCheck()); + services.AddScoped(_ => new RecordingCheck("never", events)); + }); + + var ex = Assert.ThrowsAsync(() => host.StartAsync()); + + Assert.That(ex.Message, Is.EqualTo("check failed")); + Assert.That(events, Is.Empty); + } + + [Test] + public void Cancelling_the_start_token_cancels_the_running_check() + { + using var cts = new CancellationTokenSource(); + using var host = Build(services => services.AddScoped(_ => new TokenCheck(t => + { + cts.Cancel(); + t.ThrowIfCancellationRequested(); + }))); + + Assert.CatchAsync(() => host.StartAsync(cts.Token)); + Assert.That(events, Is.Empty); + } + + [Test] + public async Task Startup_succeeds_without_checks() + { + using var host = Build(_ => { }); + + await host.StartAsync(); + + Assert.That(events, Is.EqualTo(new[] { "later service started" })); + } + + RecordingCheck Resolve(IServiceProvider provider, string name) + { + provider.GetRequiredService(); + return new RecordingCheck(name, events); + } + + IHost Build(Action configure) + { + var builder = Host.CreateEmptyApplicationBuilder(new HostApplicationBuilderSettings()); + configure(builder.Services); + builder.Services.AddStartupChecks(); + builder.Services.AddSingleton(new LaterService(events)); + return builder.Build(); + } + + class ScopedDependency : IAsyncDisposable + { + readonly List events; + + public ScopedDependency(List events) + { + this.events = events; + events.Add("scope created"); + } + + public ValueTask DisposeAsync() + { + events.Add("scope disposed"); + return ValueTask.CompletedTask; + } + } + + class RecordingCheck(string name, List events) : IStartupCheck + { + public Task Check(CancellationToken cancellationToken = default) + { + events.Add(name); + return Task.CompletedTask; + } + } + + class ThrowingCheck : IStartupCheck + { + public Task Check(CancellationToken cancellationToken = default) => throw new InvalidOperationException("check failed"); + } + + class TokenCheck(Action onCheck) : IStartupCheck + { + public Task Check(CancellationToken cancellationToken = default) + { + onCheck(cancellationToken); + return Task.CompletedTask; + } + } + + class LaterService(List events) : IHostedService + { + public Task StartAsync(CancellationToken cancellationToken = default) + { + events.Add("later service started"); + return Task.CompletedTask; + } + + public Task StopAsync(CancellationToken cancellationToken = default) => Task.CompletedTask; + } +} diff --git a/src/ServiceControl.Infrastructure/StartupChecks/IStartupCheck.cs b/src/ServiceControl.Infrastructure/StartupChecks/IStartupCheck.cs new file mode 100644 index 0000000000..666e7c5d53 --- /dev/null +++ b/src/ServiceControl.Infrastructure/StartupChecks/IStartupCheck.cs @@ -0,0 +1,9 @@ +namespace ServiceControl.Infrastructure.StartupChecks; + +using System.Threading; +using System.Threading.Tasks; + +public interface IStartupCheck +{ + Task Check(CancellationToken cancellationToken = default); +} diff --git a/src/ServiceControl.Persistence.EFCore/Abstractions/BasePersistence.cs b/src/ServiceControl.Persistence.EFCore/Abstractions/BasePersistence.cs index 4b76e5add7..1e0a590cdb 100644 --- a/src/ServiceControl.Persistence.EFCore/Abstractions/BasePersistence.cs +++ b/src/ServiceControl.Persistence.EFCore/Abstractions/BasePersistence.cs @@ -6,6 +6,7 @@ namespace ServiceControl.Persistence.EFCore.Abstractions; using Particular.LicensingComponent.Contracts; using Particular.LicensingComponent.Persistence; using ServiceControl.CustomChecks; +using ServiceControl.Infrastructure.StartupChecks; using ServiceControl.Operations.BodyStorage; using ServiceControl.Persistence.EFCore.Implementation; using ServiceControl.Persistence.EFCore.Implementation.BodyStorage; @@ -25,6 +26,8 @@ protected static void RegisterDataStores(IServiceCollection services, EFPersiste { services.TryAddSingleton(TimeProvider.System); + services.AddScoped(); + services.AddSingleton(); services.AddSingleton(); @@ -90,6 +93,7 @@ static void RegisterBodyStorage(IServiceCollection services, EFPersisterSettings services.TryAddSingleton(fileSystem); services.AddSingleton(); services.AddSingleton(); + services.AddScoped(); services.AddCustomCheck(); break; case AzureBlobBodyStorageSettings azureBlob: diff --git a/src/ServiceControl.Persistence.EFCore/Implementation/BodyStorage/FileSystemBodyStorageInstaller.cs b/src/ServiceControl.Persistence.EFCore/Implementation/BodyStorage/FileSystemBodyStorageInstaller.cs index 9c6bebceb1..2766e08e3d 100644 --- a/src/ServiceControl.Persistence.EFCore/Implementation/BodyStorage/FileSystemBodyStorageInstaller.cs +++ b/src/ServiceControl.Persistence.EFCore/Implementation/BodyStorage/FileSystemBodyStorageInstaller.cs @@ -5,24 +5,10 @@ namespace ServiceControl.Persistence.EFCore.Implementation.BodyStorage; public class FileSystemBodyStorageInstaller(FileSystemBodyStorageSettings settings) : IBodyStorageInstaller { - public async Task Provision(CancellationToken cancellationToken = default) + public Task Provision(CancellationToken cancellationToken = default) { - try - { - Directory.CreateDirectory(settings.StoragePath); + Directory.CreateDirectory(settings.StoragePath); - var probePath = Path.Combine(settings.StoragePath, $".write-probe-{Guid.NewGuid():N}"); - await using var probe = new FileStream(probePath, FileMode.CreateNew, FileAccess.Write, FileShare.None, 4096, FileOptions.DeleteOnClose); - await probe.WriteAsync(new byte[] { 1 }, cancellationToken).ConfigureAwait(false); - await probe.FlushAsync(cancellationToken).ConfigureAwait(false); - } - catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) - { - throw; - } - catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) - { - throw new InvalidOperationException($"Message body storage path '{settings.StoragePath}' is not writable. Ensure the directory can be created and the service account can write to it.", ex); - } + return Task.CompletedTask; } } diff --git a/src/ServiceControl.Persistence.EFCore/Implementation/BodyStorage/FileSystemBodyStorageStartupCheck.cs b/src/ServiceControl.Persistence.EFCore/Implementation/BodyStorage/FileSystemBodyStorageStartupCheck.cs new file mode 100644 index 0000000000..2dca500f77 --- /dev/null +++ b/src/ServiceControl.Persistence.EFCore/Implementation/BodyStorage/FileSystemBodyStorageStartupCheck.cs @@ -0,0 +1,26 @@ +namespace ServiceControl.Persistence.EFCore.Implementation.BodyStorage; + +using ServiceControl.Infrastructure.StartupChecks; +using ServiceControl.Persistence.EFCore.Abstractions; + +sealed class FileSystemBodyStorageStartupCheck(FileSystemBodyStorageSettings settings) : IStartupCheck +{ + public async Task Check(CancellationToken cancellationToken = default) + { + try + { + var probePath = Path.Combine(settings.StoragePath, $".write-probe-{Guid.NewGuid():N}"); + await using var probe = new FileStream(probePath, FileMode.CreateNew, FileAccess.Write, FileShare.None, 4096, FileOptions.DeleteOnClose); + await probe.WriteAsync(new byte[] { 1 }, cancellationToken).ConfigureAwait(false); + await probe.FlushAsync(cancellationToken).ConfigureAwait(false); + } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + { + throw; + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + throw new InvalidOperationException($"Message body storage path '{settings.StoragePath}' is not writable. Ensure the directory exists and the service account can write to it. Run --setup on the normal instance to create the directory.", ex); + } + } +} diff --git a/src/ServiceControl.Persistence.EFCore/Implementation/PendingMigrationsStartupCheck.cs b/src/ServiceControl.Persistence.EFCore/Implementation/PendingMigrationsStartupCheck.cs new file mode 100644 index 0000000000..92c5eb33a8 --- /dev/null +++ b/src/ServiceControl.Persistence.EFCore/Implementation/PendingMigrationsStartupCheck.cs @@ -0,0 +1,18 @@ +namespace ServiceControl.Persistence.EFCore.Implementation; + +using Microsoft.EntityFrameworkCore; +using ServiceControl.Infrastructure.StartupChecks; +using ServiceControl.Persistence.EFCore.DbContexts; + +sealed class PendingMigrationsStartupCheck(ServiceControlDbContext dbContext) : IStartupCheck +{ + public async Task Check(CancellationToken cancellationToken = default) + { + var pending = (await dbContext.Database.GetPendingMigrationsAsync(cancellationToken).ConfigureAwait(false)).ToList(); + + if (pending.Count > 0) + { + throw new InvalidOperationException($"The database has pending migrations: {string.Join(", ", pending)}. Run --setup on the normal instance to apply them."); + } + } +} diff --git a/src/ServiceControl.Persistence.Tests.PostgreSql/SchemaSetupTests.cs b/src/ServiceControl.Persistence.Tests.PostgreSql/SchemaSetupTests.cs index ddf899f5c8..fd548e81e5 100644 --- a/src/ServiceControl.Persistence.Tests.PostgreSql/SchemaSetupTests.cs +++ b/src/ServiceControl.Persistence.Tests.PostgreSql/SchemaSetupTests.cs @@ -9,6 +9,8 @@ namespace ServiceControl.Persistence.Tests; using Microsoft.Extensions.Logging; using NUnit.Framework; using ServiceControl.Persistence.EFCore.Abstractions; +using ServiceControl.Persistence.EFCore.DbContexts; +using ServiceControl.Persistence.EFCore.Implementation; [TestFixture] class SchemaSetupTests @@ -68,6 +70,38 @@ public async Task Migration_ignores_the_audit_migrations_history_table_in_anothe } } + [Test] + public async Task Pending_migrations_fail_the_startup_check_until_setup_has_applied_them() + { + var settings = await CreateSettings($"sc_pending_{Guid.NewGuid():n}"); + + try + { + var exception = Assert.ThrowsAsync(() => CheckPendingMigrations(settings)); + Assert.That(exception.Message, Does.Contain("pending migrations").And.Contain("--setup")); + + await ApplyMigrations(settings); + + Assert.DoesNotThrowAsync(() => CheckPendingMigrations(settings)); + } + finally + { + await TestSchema.Drop(settings.ConnectionString, settings.Schema); + } + } + + static async Task CheckPendingMigrations(PostgreSqlPersisterSettings settings) + { + var services = new ServiceCollection(); + services.AddLogging(); + new PostgreSqlPersistenceConfiguration().Create(settings).AddInstaller(services); + + await using var provider = services.BuildServiceProvider(); + using var scope = provider.CreateScope(); + + await new PendingMigrationsStartupCheck(scope.ServiceProvider.GetRequiredService()).Check(); + } + static async Task CreateSettings(string schema) => new() { ConnectionString = await PostgreSqlSharedContainer.GetConnectionStringAsync(), diff --git a/src/ServiceControl.Persistence.Tests.SqlServer/SchemaSetupTests.cs b/src/ServiceControl.Persistence.Tests.SqlServer/SchemaSetupTests.cs index bd75b98896..a667598d4a 100644 --- a/src/ServiceControl.Persistence.Tests.SqlServer/SchemaSetupTests.cs +++ b/src/ServiceControl.Persistence.Tests.SqlServer/SchemaSetupTests.cs @@ -9,6 +9,8 @@ namespace ServiceControl.Persistence.Tests; using Microsoft.Extensions.Logging; using NUnit.Framework; using ServiceControl.Persistence.EFCore.Abstractions; +using ServiceControl.Persistence.EFCore.DbContexts; +using ServiceControl.Persistence.EFCore.Implementation; [TestFixture] class SchemaSetupTests @@ -68,6 +70,38 @@ public async Task Migration_ignores_the_audit_migrations_history_table_in_anothe } } + [Test] + public async Task Pending_migrations_fail_the_startup_check_until_setup_has_applied_them() + { + var settings = await CreateSettings($"sc_pending_{Guid.NewGuid():n}"); + + try + { + var exception = Assert.ThrowsAsync(() => CheckPendingMigrations(settings)); + Assert.That(exception.Message, Does.Contain("pending migrations").And.Contain("--setup")); + + await ApplyMigrations(settings); + + Assert.DoesNotThrowAsync(() => CheckPendingMigrations(settings)); + } + finally + { + await TestSchema.Drop(settings.ConnectionString, settings.Schema); + } + } + + static async Task CheckPendingMigrations(SqlServerPersisterSettings settings) + { + var services = new ServiceCollection(); + services.AddLogging(); + new SqlServerPersistenceConfiguration().Create(settings).AddInstaller(services); + + await using var provider = services.BuildServiceProvider(); + using var scope = provider.CreateScope(); + + await new PendingMigrationsStartupCheck(scope.ServiceProvider.GetRequiredService()).Check(); + } + static async Task CreateSettings(string schema) => new() { ConnectionString = await SqlServerSharedContainer.GetConnectionStringAsync(), diff --git a/src/ServiceControl.Persistence.Tests/EFCore/BodyStoragePersistenceTests.cs b/src/ServiceControl.Persistence.Tests/EFCore/BodyStoragePersistenceTests.cs index c3f9db1b6d..bcfd45e356 100644 --- a/src/ServiceControl.Persistence.Tests/EFCore/BodyStoragePersistenceTests.cs +++ b/src/ServiceControl.Persistence.Tests/EFCore/BodyStoragePersistenceTests.cs @@ -195,12 +195,38 @@ public async Task Provisioning_keeps_existing_bodies() } [Test] - public async Task Provisioning_fails_with_the_path_when_it_is_a_file() + public async Task Startup_check_keeps_existing_bodies_and_removes_its_probe() + { + var existing = Path.Combine(tempDir, "existing"); + await File.WriteAllTextAsync(existing, "body"); + + await new FileSystemBodyStorageStartupCheck(new FileSystemBodyStorageSettings { StoragePath = tempDir }).Check(); + + Assert.That(Directory.GetFileSystemEntries(tempDir), Is.EqualTo(new[] { existing })); + } + + [Test] + public void Startup_check_fails_without_creating_a_missing_directory() + { + var path = Path.Combine(tempDir, "missing"); + + var ex = Assert.ThrowsAsync(() => new FileSystemBodyStorageStartupCheck(new FileSystemBodyStorageSettings { StoragePath = path }).Check()); + + using (Assert.EnterMultipleScope()) + { + Assert.That(ex.Message, Does.Contain(path).And.Contain("--setup")); + Assert.That(ex.InnerException, Is.InstanceOf()); + Assert.That(Directory.Exists(path), Is.False); + } + } + + [Test] + public async Task Startup_check_fails_with_the_path_when_it_is_a_file() { var path = Path.Combine(tempDir, "file"); await File.WriteAllTextAsync(path, "x"); - var ex = Assert.ThrowsAsync(() => new FileSystemBodyStorageInstaller(new FileSystemBodyStorageSettings { StoragePath = path }).Provision()); + var ex = Assert.ThrowsAsync(() => new FileSystemBodyStorageStartupCheck(new FileSystemBodyStorageSettings { StoragePath = path }).Check()); using (Assert.EnterMultipleScope()) { @@ -211,7 +237,7 @@ public async Task Provisioning_fails_with_the_path_when_it_is_a_file() [Test] [UnsupportedOSPlatform("windows")] - public void Provisioning_fails_with_the_path_when_the_directory_is_not_writable() + public async Task Read_only_directory_is_provisioned_but_fails_the_startup_check() { if (OperatingSystem.IsWindows()) { @@ -229,7 +255,10 @@ public void Provisioning_fails_with_the_path_when_the_directory_is_not_writable( Assert.Ignore("The current account bypasses directory permissions."); } - var ex = Assert.ThrowsAsync(() => new FileSystemBodyStorageInstaller(new FileSystemBodyStorageSettings { StoragePath = path }).Provision()); + var settings = new FileSystemBodyStorageSettings { StoragePath = path }; + + await new FileSystemBodyStorageInstaller(settings).Provision(); + var ex = Assert.ThrowsAsync(() => new FileSystemBodyStorageStartupCheck(settings).Check()); using (Assert.EnterMultipleScope()) { diff --git a/src/ServiceControl.Persistence.Tests/EFCore/FileSystemBodyStorageCustomCheckTests.cs b/src/ServiceControl.Persistence.Tests/EFCore/FileSystemBodyStorageCustomCheckTests.cs index 835dace309..1f09c4d550 100644 --- a/src/ServiceControl.Persistence.Tests/EFCore/FileSystemBodyStorageCustomCheckTests.cs +++ b/src/ServiceControl.Persistence.Tests/EFCore/FileSystemBodyStorageCustomCheckTests.cs @@ -9,7 +9,9 @@ namespace ServiceControl.Persistence.Tests; using Microsoft.Extensions.Logging.Abstractions; using NServiceBus.CustomChecks; using NUnit.Framework; +using ServiceControl.Infrastructure.StartupChecks; using ServiceControl.Persistence.EFCore.Abstractions; +using ServiceControl.Persistence.EFCore.Implementation; using ServiceControl.Persistence.EFCore.Implementation.BodyStorage; [TestFixture] @@ -140,7 +142,25 @@ public void Is_registered_and_resolvable_for_file_system_body_storage() var check = services.BuildServiceProvider().GetServices().OfType().SingleOrDefault(); - Assert.That(check, Is.Not.Null); + using (Assert.EnterMultipleScope()) + { + Assert.That(check, Is.Not.Null); + Assert.That(services.Any(descriptor => descriptor.ServiceType == typeof(IStartupCheck) && descriptor.ImplementationType == typeof(PendingMigrationsStartupCheck) && descriptor.Lifetime == ServiceLifetime.Scoped), Is.True); + Assert.That(services.Any(descriptor => descriptor.ServiceType == typeof(IStartupCheck) && descriptor.ImplementationType == typeof(FileSystemBodyStorageStartupCheck) && descriptor.Lifetime == ServiceLifetime.Scoped), Is.True); + } + } + + [Test] + public void Setup_does_not_register_startup_checks() + { + var services = new ServiceCollection(); + RegisterBodyStorageInstaller(services, new TestPersisterSettings + { + ConnectionString = "Server=nowhere", + BodyStorage = CreateSettings(StoragePath, 15) + }); + + Assert.That(services.Any(descriptor => descriptor.ServiceType == typeof(IStartupCheck)), Is.False); } [Test] @@ -153,7 +173,11 @@ public void Is_not_registered_for_other_body_storage_types() BodyStorage = new S3BodyStorageSettings { BucketName = "bodies" } }); - Assert.That(services.Any(descriptor => descriptor.ImplementationType == typeof(FileSystemBodyStorageCustomCheck)), Is.False); + using (Assert.EnterMultipleScope()) + { + Assert.That(services.Any(descriptor => descriptor.ImplementationType == typeof(FileSystemBodyStorageCustomCheck)), Is.False); + Assert.That(services.Any(descriptor => descriptor.ImplementationType == typeof(FileSystemBodyStorageStartupCheck)), Is.False); + } } [Test] diff --git a/src/ServiceControl/HostApplicationBuilderExtensions.cs b/src/ServiceControl/HostApplicationBuilderExtensions.cs index 647b549024..c294001cf3 100644 --- a/src/ServiceControl/HostApplicationBuilderExtensions.cs +++ b/src/ServiceControl/HostApplicationBuilderExtensions.cs @@ -105,6 +105,7 @@ public static void AddServiceControl(this IHostApplicationBuilder hostBuilder, S // directly and to make things more complex of course the order of registration still matters ;) services.AddSingleton(provider => new Lazy(provider.GetRequiredService)); + services.AddStartupChecks(); services.AddPersistence(settings); services.AddMetrics(settings.PrintMetrics); hostBuilder.AddTelemetry(settings); From 91159b28ba80e45c88f36bc31b07fcaf17bb27a0 Mon Sep 17 00:00:00 2001 From: Rhys Bevilaqua Date: Mon, 2 Nov 2026 12:22:51 +0800 Subject: [PATCH 3/3] Update log messages --- .../Implementation/PendingMigrationsStartupCheck.cs | 2 +- .../Implementation/PendingMigrationsStartupCheck.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ServiceControl.Audit.Persistence.EFCore/Implementation/PendingMigrationsStartupCheck.cs b/src/ServiceControl.Audit.Persistence.EFCore/Implementation/PendingMigrationsStartupCheck.cs index 1737256f08..196f9d6328 100644 --- a/src/ServiceControl.Audit.Persistence.EFCore/Implementation/PendingMigrationsStartupCheck.cs +++ b/src/ServiceControl.Audit.Persistence.EFCore/Implementation/PendingMigrationsStartupCheck.cs @@ -12,7 +12,7 @@ public async Task Check(CancellationToken cancellationToken = default) if (pending.Count > 0) { - throw new InvalidOperationException($"The audit database has pending migrations: {string.Join(", ", pending)}. Run the instance with --setup to apply them."); + throw new InvalidOperationException($"The audit database has pending migrations. Run the instance with --setup to apply them."); } } } diff --git a/src/ServiceControl.Persistence.EFCore/Implementation/PendingMigrationsStartupCheck.cs b/src/ServiceControl.Persistence.EFCore/Implementation/PendingMigrationsStartupCheck.cs index 92c5eb33a8..0057e1cfde 100644 --- a/src/ServiceControl.Persistence.EFCore/Implementation/PendingMigrationsStartupCheck.cs +++ b/src/ServiceControl.Persistence.EFCore/Implementation/PendingMigrationsStartupCheck.cs @@ -12,7 +12,7 @@ public async Task Check(CancellationToken cancellationToken = default) if (pending.Count > 0) { - throw new InvalidOperationException($"The database has pending migrations: {string.Join(", ", pending)}. Run --setup on the normal instance to apply them."); + throw new InvalidOperationException($"The database has pending migrations. Run the instance with --setup to apply them."); } } }