diff --git a/Emby.Server.Implementations/ApplicationHost.cs b/Emby.Server.Implementations/ApplicationHost.cs index 5f42a24825..e785aa4ebf 100644 --- a/Emby.Server.Implementations/ApplicationHost.cs +++ b/Emby.Server.Implementations/ApplicationHost.cs @@ -124,6 +124,12 @@ namespace Emby.Server.Implementations /// public abstract class ApplicationHost : IServerApplicationHost, IDisposable { + /// + /// The secret the startup read of the quick connect store looks for. No flow ever mints it, so the + /// read is always a miss and only its reachability is being asked about. + /// + private const string StartupProbeSecret = "startup-probe"; + /// /// The disposable parts. /// @@ -644,6 +650,8 @@ namespace Emby.Server.Implementations /// A task representing the service initialization operation. public async Task InitializeServices(IConfiguration startupConfig) { + await ProbeQuickConnectStoreAsync().ConfigureAwait(false); + var localizationManager = (LocalizationManager)Resolve(); await localizationManager.LoadAll().ConfigureAwait(false); @@ -652,6 +660,27 @@ namespace Emby.Server.Implementations FindParts(); } + /// + /// Reads the quick connect store once here so a store that cannot be reached stops startup, rather + /// than being discovered on the first request that needs it. A read rather than a resolve because a + /// shared store built with abortConnect=false constructs without touching the network. + /// + private async Task ProbeQuickConnectStoreAsync() + { + try + { + await Resolve().GetRequestBySecretAsync(StartupProbeSecret).ConfigureAwait(false); + } + catch (Exception ex) + { + Logger.LogCritical( + ex, + "Quick connect is configured against the shared valkey/Redis store at {Key} and it is UNREACHABLE, so the server will not start. Bring valkey up, or clear that setting to keep quick connect state on this instance alone.", + TranscodeStoreOptions.RedisConnectionStringKey); + throw; + } + } + private X509Certificate2 GetCertificate(string path, string password) { if (string.IsNullOrWhiteSpace(path)) diff --git a/Jellyfin.Server/Extensions/QuickConnectStoreServiceCollectionExtensions.cs b/Jellyfin.Server/Extensions/QuickConnectStoreServiceCollectionExtensions.cs index aa13634a9e..db7bf9119c 100644 --- a/Jellyfin.Server/Extensions/QuickConnectStoreServiceCollectionExtensions.cs +++ b/Jellyfin.Server/Extensions/QuickConnectStoreServiceCollectionExtensions.cs @@ -21,8 +21,9 @@ public static class QuickConnectStoreServiceCollectionExtensions /// /// The connection string is only set for a multi-instance deployment, which is the only shape where /// the initiate, authorize and exchange legs of one flow can land on different instances. Set but - /// unreachable is a misconfigured deployment rather than a single-instance one, so it fails rather - /// than quietly handing out a store the other instances cannot see. + /// unreachable is a misconfigured deployment rather than a single-instance one, so the store is read + /// once during startup and an unreachable one stops the server coming up, rather than quietly handing + /// out a store the other instances cannot see. /// /// The service collection. /// The configuration to read the Redis connection string from. diff --git a/tests/Jellyfin.Server.Tests/QuickConnect/QuickConnectStartupTests.cs b/tests/Jellyfin.Server.Tests/QuickConnect/QuickConnectStartupTests.cs new file mode 100644 index 0000000000..ea96960f52 --- /dev/null +++ b/tests/Jellyfin.Server.Tests/QuickConnect/QuickConnectStartupTests.cs @@ -0,0 +1,264 @@ +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Threading.Tasks; +using Emby.Server.Implementations; +using Emby.Server.Implementations.QuickConnect; +using Jellyfin.Server.Extensions; +using Jellyfin.Server.Helpers; +using Jellyfin.Server.Migrations.Stages; +using Jellyfin.Server.ServerSetupApp; +using Jellyfin.Server.Tests.HighAvailability; +using MediaBrowser.Common.Configuration; +using MediaBrowser.Common.Extensions; +using MediaBrowser.Controller.Net; +using MediaBrowser.Controller.QuickConnect; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; +using StackExchange.Redis; +using Xunit; + +namespace Jellyfin.Server.Tests.QuickConnect; + +/// +/// Brings the server up the way Program does - the real host over , the +/// startup and core migrations, then - to pin down what +/// a pod does when the quick connect store it is configured against cannot be reached. +/// +[Trait("Category", "RequiresDocker")] +[Collection("JellyfinSectionConfiguration")] +public sealed class QuickConnectStartupTests : IAsyncLifetime +{ + private const string RedisConnectionStringVariable = "Jellyfin__TranscodeStore__RedisConnectionString"; + + private RedisTestServer _redis = null!; + + /// + public async ValueTask InitializeAsync() + { + _redis = await RedisTestServer.StartAsync().ConfigureAwait(false); + } + + /// + public async ValueTask DisposeAsync() + { + Environment.SetEnvironmentVariable(RedisConnectionStringVariable, null); + await _redis.DisposeAsync().ConfigureAwait(false); + } + + /// + /// A configured but unreachable store stops the server coming up, so the outage is visible where the + /// server is started instead of arriving later as a failure on every request that needs the store. The + /// connection string carries the abortConnect=false a deployment uses, so the multiplexer + /// connects lazily and only a real read settles whether the store can be served. + /// + [Fact] + public void UnreachableStore_StopsStartup() + { + Environment.SetEnvironmentVariable( + RedisConnectionStringVariable, + "127.0.0.1:1,abortConnect=false,connectTimeout=250,connectRetry=0,syncTimeout=250"); + + using var server = new StartupHarness(); + + Assert.ThrowsAny(() => server.Services); + Assert.Contains( + server.CriticalEntries, + entry => entry.Contains("Quick connect", StringComparison.Ordinal) + && entry.Contains("UNREACHABLE", StringComparison.Ordinal) + && entry.Contains("valkey", StringComparison.Ordinal)); + } + + /// + /// A reachable store lets the server come up, and the quick connect it comes up with holds its + /// requests in the shared store every instance reads. + /// + /// A representing the asynchronous operation. + [Fact] + public async Task ReachableStore_StartsAndServesQuickConnect() + { + Environment.SetEnvironmentVariable(RedisConnectionStringVariable, _redis.ConnectionString); + + using var server = new StartupHarness(); + + Assert.IsType(server.Services.GetRequiredService()); + + var quickConnect = server.Services.GetRequiredService(); + var request = await quickConnect.TryConnect(NewAuthorizationInfo()); + + Assert.Equal(request.Code, (await quickConnect.CheckRequestStatus(request.Secret)).Code); + + await using var redis = await _redis.ConnectAsync(); + Assert.True(await redis.GetDatabase().KeyExistsAsync("jellyfin:quickconnect:request:" + request.Secret)); + } + + /// + /// Without a connection string the deployment is single-instance, and it starts on the process-local + /// store upstream uses. + /// + /// A representing the asynchronous operation. + [Fact] + public async Task NoConnectionString_StartsOnTheProcessLocalStore() + { + Environment.SetEnvironmentVariable(RedisConnectionStringVariable, null); + + using var server = new StartupHarness(); + + Assert.IsType(server.Services.GetRequiredService()); + + var quickConnect = server.Services.GetRequiredService(); + var request = await quickConnect.TryConnect(NewAuthorizationInfo()); + + Assert.Equal(request.Code, (await quickConnect.CheckRequestStatus(request.Secret)).Code); + } + + private static AuthorizationInfo NewAuthorizationInfo() => new AuthorizationInfo + { + DeviceId = Guid.NewGuid().ToString("N"), + Device = "Living Room TV", + Client = "Jellyfin Web", + Version = "1.0.0" + }; + + private sealed class StartupHarness : WebApplicationFactory + { + private readonly ConcurrentBag _disposables = new(); + private readonly ConcurrentQueue _criticalEntries = new(); + private readonly string _root = Path.Combine( + Path.GetTempPath(), + "jellyfin-quickconnect-startup", + Path.GetRandomFileName()); + + static StartupHarness() + { + StartupHelpers.PerformStaticInitialization(); + } + + public IReadOnlyCollection CriticalEntries => _criticalEntries.ToArray(); + + protected override IHostBuilder CreateHostBuilder() => new HostBuilder(); + + protected override void ConfigureWebHost(IWebHostBuilder builder) + { + Environment.SetEnvironmentVariable("JELLYFIN_FFMPEG__NOVALIDATION", "true"); + + var commandLineOpts = new StartupOptions(); + Directory.CreateDirectory(Path.Combine(_root, "logs")); + Directory.CreateDirectory(Path.Combine(_root, "config")); + Directory.CreateDirectory(Path.Combine(_root, "cache")); + Directory.CreateDirectory(Path.Combine(_root, "jellyfin-web")); + var appPaths = new ServerApplicationPaths( + _root, + Path.Combine(_root, "logs"), + Path.Combine(_root, "config"), + Path.Combine(_root, "cache"), + Path.Combine(_root, "jellyfin-web")); + + StartupHelpers.InitLoggingConfigFile(appPaths).GetAwaiter().GetResult(); + + var startupConfig = Program.CreateAppConfiguration(commandLineOpts, appPaths); + + ILoggerFactory loggerFactory = LoggerFactory.Create(logging => logging + .SetMinimumLevel(LogLevel.Critical) + .AddProvider(new CriticalEntryProvider(_criticalEntries))); + _disposables.Add(loggerFactory); + + var appHost = new CoreAppHost(appPaths, loggerFactory, commandLineOpts, startupConfig); + _disposables.Add(appHost); + + builder.ConfigureServices(services => appHost.Init(services)) + .ConfigureWebHostBuilder(appHost, startupConfig, appPaths, NullLogger.Instance) + .ConfigureAppConfiguration((context, configuration) => configuration + .SetBasePath(appPaths.ConfigurationDirectoryPath) + .AddInMemoryCollection(Emby.Server.Implementations.ConfigurationOptions.DefaultConfiguration) + .AddEnvironmentVariables("JELLYFIN_") + .AddInMemoryCollection(commandLineOpts.ConvertToConfig())) + .ConfigureServices(services => services.RegisterStartupLogger()); + } + + protected override IHost CreateHost(IHostBuilder builder) + { + var host = builder.Build(); + var appHost = (CoreAppHost)host.Services.GetRequiredService(); + appHost.ServiceProvider = host.Services; + var appPaths = (ServerApplicationPaths)host.Services.GetRequiredService(); + var configuration = host.Services.GetRequiredService(); + + Program.ApplyStartupMigrationAsync(appPaths, configuration, new StartupOptions()).GetAwaiter().GetResult(); + Program.ApplyCoreMigrationsAsync(host.Services, JellyfinMigrationStageTypes.CoreInitialisation).GetAwaiter().GetResult(); + appHost.InitializeServices(configuration).GetAwaiter().GetResult(); + Program.ApplyCoreMigrationsAsync(host.Services, JellyfinMigrationStageTypes.AppInitialisation).GetAwaiter().GetResult(); + host.Start(); + + return host; + } + + protected override void Dispose(bool disposing) + { + base.Dispose(disposing); + + foreach (var disposable in _disposables) + { + disposable.Dispose(); + } + + _disposables.Clear(); + + try + { + Directory.Delete(_root, true); + } + catch (IOException) + { + // A temporary directory left behind is not worth failing a test over. + } + } + + private sealed class CriticalEntryProvider : ILoggerProvider + { + private readonly ConcurrentQueue _entries; + + public CriticalEntryProvider(ConcurrentQueue entries) + { + _entries = entries; + } + + public ILogger CreateLogger(string categoryName) => new CriticalEntryLogger(_entries); + + public void Dispose() + { + } + + private sealed class CriticalEntryLogger : ILogger + { + private readonly ConcurrentQueue _entries; + + public CriticalEntryLogger(ConcurrentQueue entries) + { + _entries = entries; + } + + public IDisposable? BeginScope(TState state) + where TState : notnull + => null; + + public bool IsEnabled(LogLevel logLevel) => logLevel == LogLevel.Critical; + + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) + { + if (logLevel == LogLevel.Critical) + { + _entries.Enqueue(formatter!(state, exception)); + } + } + } + } + } +} diff --git a/tests/Jellyfin.Server.Tests/QuickConnect/QuickConnectStoreWiringTests.cs b/tests/Jellyfin.Server.Tests/QuickConnect/QuickConnectStoreWiringTests.cs index b492f44d98..f16fee5c40 100644 --- a/tests/Jellyfin.Server.Tests/QuickConnect/QuickConnectStoreWiringTests.cs +++ b/tests/Jellyfin.Server.Tests/QuickConnect/QuickConnectStoreWiringTests.cs @@ -108,9 +108,9 @@ public sealed class QuickConnectStoreWiringTests : IAsyncLifetime } /// - /// A connection string that is set but unreachable is a misconfigured multi-instance deployment. It - /// fails rather than handing out a store the other instances cannot see, which would put quick - /// connect back on the cross-instance behaviour this configuration exists to fix. + /// An unreachable connection string that connects eagerly, the default, cannot even build the store. + /// The lazily connecting form a deployment uses builds one, and the startup read in + /// is what stops the server coming up on that. /// [Fact] public void UnreachableRedisAtStartup_FailsClosed()