Merge pull request #1874 from tgstation/FixConfigReloadAtSetup

Reload configuration after successfully running the setup wizard

src/Tgstation.Server.Host/ServerFactory.cs

@@ -125,15 +125,22 @@ IHostBuilder CreateDefaultBuilder() => Microsoft.Extensions.Hosting.Host.CreateD
 			var setupWizardHostBuilder = CreateDefaultBuilder()
 				.UseSetupApplication(assemblyInformationProvider, IOManager);
 
-			IPostSetupServices<ServerFactory> postSetupServices;
+			IPostSetupServices postSetupServices;
 			using (var setupHost = setupWizardHostBuilder.Build())
 			{
-				postSetupServices = setupHost.Services.GetRequiredService<IPostSetupServices<ServerFactory>>();
+				ILogger<ServerFactory> logger = setupHost.Services.GetRequiredService<ILogger<ServerFactory>>();
+				postSetupServices = setupHost.Services.GetRequiredService<IPostSetupServices>();
 				await setupHost.RunAsync(cancellationToken);
 
 				if (postSetupServices.GeneralConfiguration.SetupWizardMode == SetupWizardMode.Only)
 				{
-					postSetupServices.Logger.LogInformation("Shutting down due to only running setup wizard.");
+					logger.LogInformation("Shutting down due to only running setup wizard.");
+					return null;
+				}
+
+				if (postSetupServices.ReloadRequired)
+				{
+					logger.LogInformation("TGS must restart to reload the updated configuration.");
 					return null;
 				}
 			}

src/Tgstation.Server.Host/Setup/IPostSetupServices.cs

@@ -42,5 +42,10 @@ public interface IPostSetupServices
 		/// The <see cref="IPlatformIdentifier"/>.
 		/// </summary>
 		IPlatformIdentifier PlatformIdentifier { get; }
+
+		/// <summary>
+		/// If an application reload to get updated configuration values is required.
+		/// </summary>
+		bool ReloadRequired { get; set; }
 	}
 }

src/Tgstation.Server.Host/Setup/PostSetupServices.cs

@@ -1,6 +1,5 @@
 using System;
 
-using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
 
 using Tgstation.Server.Host.Configuration;
@@ -9,14 +8,11 @@
 namespace Tgstation.Server.Host.Setup
 {
 	/// <inheritdoc />
-	sealed class PostSetupServices<TLoggerType> : IPostSetupServices<TLoggerType>
+	sealed class PostSetupServices : IPostSetupServices
 	{
 		/// <inheritdoc />
 		public IPlatformIdentifier PlatformIdentifier { get; }
 
-		/// <inheritdoc />
-		public ILogger<TLoggerType> Logger { get; }
-
 		/// <inheritdoc />
 		public GeneralConfiguration GeneralConfiguration => generalConfigurationOptions.Value;
 
@@ -35,6 +31,9 @@ sealed class PostSetupServices<TLoggerType> : IPostSetupServices<TLoggerType>
 		/// <inheritdoc />
 		public ElasticsearchConfiguration ElasticsearchConfiguration => elasticsearchConfigurationOptions.Value;
 
+		/// <inheritdoc />
+		public bool ReloadRequired { get; set; }
+
 		/// <summary>
 		/// Backing <see cref="IOptions{TOptions}"/> for <see cref="GeneralConfiguration"/>.
 		/// </summary>
@@ -66,10 +65,9 @@ sealed class PostSetupServices<TLoggerType> : IPostSetupServices<TLoggerType>
 		readonly IOptions<InternalConfiguration> internalConfigurationOptions;
 
 		/// <summary>
-		/// Initializes a new instance of the <see cref="PostSetupServices{TLoggerType}"/> class.
+		/// Initializes a new instance of the <see cref="PostSetupServices"/> class.
 		/// </summary>
 		/// <param name="platformIdentifier">The value of <see cref="PlatformIdentifier"/>.</param>
-		/// <param name="loggerFactory">The <see cref="ILoggerFactory"/> used to create <see cref="Logger"/>.</param>
 		/// <param name="generalConfigurationOptions">The <see cref="IOptions{TOptions}"/> containing the value of <see cref="GeneralConfiguration"/>.</param>
 		/// <param name="databaseConfigurationOptions">The <see cref="IOptions{TOptions}"/> containing the value of <see cref="DatabaseConfiguration"/>.</param>
 		/// <param name="securityConfigurationOptions">The <see cref="IOptions{TOptions}"/> containing the value of <see cref="SecurityConfiguration"/>.</param>
@@ -78,7 +76,6 @@ sealed class PostSetupServices<TLoggerType> : IPostSetupServices<TLoggerType>
 		/// <param name="internalConfigurationOptions">The <see cref="IOptions{TOptions}"/> containing the value of <see cref="InternalConfiguration"/>.</param>
 		public PostSetupServices(
 			IPlatformIdentifier platformIdentifier,
-			ILoggerFactory loggerFactory,
 			IOptions<GeneralConfiguration> generalConfigurationOptions,
 			IOptions<DatabaseConfiguration> databaseConfigurationOptions,
 			IOptions<SecurityConfiguration> securityConfigurationOptions,
@@ -87,9 +84,6 @@ public PostSetupServices(
 			IOptions<InternalConfiguration> internalConfigurationOptions)
 		{
 			PlatformIdentifier = platformIdentifier ?? throw new ArgumentNullException(nameof(platformIdentifier));
-			ArgumentNullException.ThrowIfNull(loggerFactory);
-
-			Logger = loggerFactory.CreateLogger<TLoggerType>();
 			this.generalConfigurationOptions = generalConfigurationOptions ?? throw new ArgumentNullException(nameof(generalConfigurationOptions));
 			this.databaseConfigurationOptions = databaseConfigurationOptions ?? throw new ArgumentNullException(nameof(databaseConfigurationOptions));
 			this.securityConfigurationOptions = securityConfigurationOptions ?? throw new ArgumentNullException(nameof(securityConfigurationOptions));

src/Tgstation.Server.Host/Setup/SetupApplication.cs

@@ -72,7 +72,7 @@ public void ConfigureServices(IServiceCollection services, IAssemblyInformationP
 		/// <param name="services">The <see cref="IServiceCollection"/> to configure.</param>
 		protected virtual void ConfigureHostedService(IServiceCollection services)
 		{
-			services.AddSingleton(typeof(IPostSetupServices<>), typeof(PostSetupServices<>));
+			services.AddSingleton<IPostSetupServices, PostSetupServices>();
 			services.AddSingleton<IHostedService, SetupWizard>();
 		}
 	}

src/Tgstation.Server.Host/Setup/SetupWizard.cs

@@ -76,6 +76,11 @@ sealed class SetupWizard : BackgroundService
 		/// </summary>
 		readonly IHostApplicationLifetime applicationLifetime;
 
+		/// <summary>
+		/// The <see cref="IPostSetupServices"/> for the <see cref="SetupWizard"/>.
+		/// </summary>
+		readonly IPostSetupServices postSetupServices;
+
 		/// <summary>
 		/// The <see cref="GeneralConfiguration"/> for the <see cref="SetupWizard"/>.
 		/// </summary>
@@ -97,6 +102,7 @@ sealed class SetupWizard : BackgroundService
 		/// <param name="platformIdentifier">The value of <see cref="platformIdentifier"/>.</param>
 		/// <param name="asyncDelayer">The value of <see cref="asyncDelayer"/>.</param>
 		/// <param name="applicationLifetime">The value of <see cref="applicationLifetime"/>.</param>
+		/// <param name="postSetupServices">The value of <see cref="postSetupServices"/>.</param>
 		/// <param name="generalConfigurationOptions">The <see cref="IOptions{TOptions}"/> containing the value of <see cref="generalConfiguration"/>.</param>
 		/// <param name="internalConfigurationOptions">The <see cref="IOptions{TOptions}"/> containing the value of <see cref="internalConfiguration"/>.</param>
 		public SetupWizard(
@@ -108,6 +114,7 @@ public SetupWizard(
 			IPlatformIdentifier platformIdentifier,
 			IAsyncDelayer asyncDelayer,
 			IHostApplicationLifetime applicationLifetime,
+			IPostSetupServices postSetupServices,
 			IOptions<GeneralConfiguration> generalConfigurationOptions,
 			IOptions<InternalConfiguration> internalConfigurationOptions)
 		{
@@ -119,6 +126,7 @@ public SetupWizard(
 			this.platformIdentifier = platformIdentifier ?? throw new ArgumentNullException(nameof(platformIdentifier));
 			this.asyncDelayer = asyncDelayer ?? throw new ArgumentNullException(nameof(asyncDelayer));
 			this.applicationLifetime = applicationLifetime ?? throw new ArgumentNullException(nameof(applicationLifetime));
+			this.postSetupServices = postSetupServices ?? throw new ArgumentNullException(nameof(postSetupServices));
 
 			generalConfiguration = generalConfigurationOptions?.Value ?? throw new ArgumentNullException(nameof(generalConfigurationOptions));
 			internalConfiguration = internalConfigurationOptions?.Value ?? throw new ArgumentNullException(nameof(internalConfigurationOptions));
@@ -1014,6 +1022,8 @@ await ioManager.WriteAllBytes(
 					userConfigFileName,
 					configBytes,
 					cancellationToken);
+
+				postSetupServices.ReloadRequired = true;
 			}
 			catch (Exception e) when (e is not OperationCanceledException)
 			{

tests/Tgstation.Server.Host.Tests/Setup/TestSetupWizard.cs

@@ -27,25 +27,27 @@ public sealed class TestSetupWizard
 		[TestMethod]
 		public void TestConstructionThrows()
 		{
-			Assert.ThrowsException<ArgumentNullException>(() => new SetupWizard(null, null, null, null, null, null, null, null, null, null));
+			Assert.ThrowsException<ArgumentNullException>(() => new SetupWizard(null, null, null, null, null, null, null, null, null, null, null));
 			var mockIOManager = new Mock<IIOManager>();
-			Assert.ThrowsException<ArgumentNullException>(() => new SetupWizard(mockIOManager.Object, null, null, null, null, null, null, null, null, null));
+			Assert.ThrowsException<ArgumentNullException>(() => new SetupWizard(mockIOManager.Object, null, null, null, null, null, null, null, null, null, null));
 			var mockConsole = new Mock<IConsole>();
-			Assert.ThrowsException<ArgumentNullException>(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, null, null, null, null, null, null, null, null));
+			Assert.ThrowsException<ArgumentNullException>(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, null, null, null, null, null, null, null, null, null));
 			var mockHostingEnvironment = new Mock<IWebHostEnvironment>();
-			Assert.ThrowsException<ArgumentNullException>(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, mockHostingEnvironment.Object, null, null, null, null, null, null, null));
+			Assert.ThrowsException<ArgumentNullException>(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, mockHostingEnvironment.Object, null, null, null, null, null, null, null, null));
 			var mockAssemblyInfoProvider = new Mock<IAssemblyInformationProvider>();
-			Assert.ThrowsException<ArgumentNullException>(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, mockHostingEnvironment.Object, mockAssemblyInfoProvider.Object, null, null, null, null, null, null));
+			Assert.ThrowsException<ArgumentNullException>(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, mockHostingEnvironment.Object, mockAssemblyInfoProvider.Object, null, null, null, null, null, null, null));
 			var mockDBConnectionFactory = new Mock<IDatabaseConnectionFactory>();
-			Assert.ThrowsException<ArgumentNullException>(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, mockHostingEnvironment.Object, mockAssemblyInfoProvider.Object, mockDBConnectionFactory.Object, null, null, null, null, null));
+			Assert.ThrowsException<ArgumentNullException>(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, mockHostingEnvironment.Object, mockAssemblyInfoProvider.Object, mockDBConnectionFactory.Object, null, null, null, null, null, null));
 			var mockPlatformIdentifier = new Mock<IPlatformIdentifier>();
-			Assert.ThrowsException<ArgumentNullException>(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, mockHostingEnvironment.Object, mockAssemblyInfoProvider.Object, mockDBConnectionFactory.Object, mockPlatformIdentifier.Object, null, null, null, null));
+			Assert.ThrowsException<ArgumentNullException>(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, mockHostingEnvironment.Object, mockAssemblyInfoProvider.Object, mockDBConnectionFactory.Object, mockPlatformIdentifier.Object, null, null, null, null, null));
 			var mockAsyncDelayer = new Mock<IAsyncDelayer>();
-			Assert.ThrowsException<ArgumentNullException>(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, mockHostingEnvironment.Object, mockAssemblyInfoProvider.Object, mockDBConnectionFactory.Object, mockPlatformIdentifier.Object, mockAsyncDelayer.Object, null, null, null));
+			Assert.ThrowsException<ArgumentNullException>(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, mockHostingEnvironment.Object, mockAssemblyInfoProvider.Object, mockDBConnectionFactory.Object, mockPlatformIdentifier.Object, mockAsyncDelayer.Object, null, null, null, null));
 			var mockLifetime = new Mock<IHostApplicationLifetime>();
-			Assert.ThrowsException<ArgumentNullException>(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, mockHostingEnvironment.Object, mockAssemblyInfoProvider.Object, mockDBConnectionFactory.Object, mockPlatformIdentifier.Object, mockAsyncDelayer.Object, mockLifetime.Object, null, null));
+			Assert.ThrowsException<ArgumentNullException>(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, mockHostingEnvironment.Object, mockAssemblyInfoProvider.Object, mockDBConnectionFactory.Object, mockPlatformIdentifier.Object, mockAsyncDelayer.Object, mockLifetime.Object, null, null, null));
+			var mockServices = new Mock<IPostSetupServices>();
+			Assert.ThrowsException<ArgumentNullException>(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, mockHostingEnvironment.Object, mockAssemblyInfoProvider.Object, mockDBConnectionFactory.Object, mockPlatformIdentifier.Object, mockAsyncDelayer.Object, mockLifetime.Object, mockServices.Object, null, null));
 			var mockGeneralConfigurationOptions = Options.Create(new GeneralConfiguration());
-			Assert.ThrowsException<ArgumentNullException>(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, mockHostingEnvironment.Object, mockAssemblyInfoProvider.Object, mockDBConnectionFactory.Object, mockPlatformIdentifier.Object, mockAsyncDelayer.Object, mockLifetime.Object, mockGeneralConfigurationOptions, null));
+			Assert.ThrowsException<ArgumentNullException>(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, mockHostingEnvironment.Object, mockAssemblyInfoProvider.Object, mockDBConnectionFactory.Object, mockPlatformIdentifier.Object, mockAsyncDelayer.Object, mockLifetime.Object, mockServices.Object, mockGeneralConfigurationOptions, null));
 		}
 
 		[TestMethod]
@@ -61,6 +63,7 @@ public async Task TestWithUserStupidity()
 			var mockInternalConfigurationOptions = new Mock<IOptions<InternalConfiguration>>();
 			var mockPlatformIdentifier = new Mock<IPlatformIdentifier>();
 			var mockAsyncDelayer = new Mock<IAsyncDelayer>();
+			var mockServices = new Mock<IPostSetupServices>();
 
 			var testGeneralConfig = new GeneralConfiguration
 			{
@@ -83,6 +86,7 @@ public async Task TestWithUserStupidity()
 				mockPlatformIdentifier.Object,
 				mockAsyncDelayer.Object,
 				mockLifetime.Object,
+				mockServices.Object,
 				mockGeneralConfigurationOptions.Object,
 				mockInternalConfigurationOptions.Object);
 

tests/Tgstation.Server.Host.Tests/TestServerFactory.cs

@@ -15,8 +15,10 @@ namespace Tgstation.Server.Host.Tests
 	[TestClass]
 	public sealed class TestServerFactory
 	{
+		static readonly string[] cliArgs = ["General:SetupWizardMode=Never"];
+
 		[TestMethod]
-		public void TestContructor()
+		public void TestConstructor()
 		{
 			Assert.ThrowsException<ArgumentNullException>(() => new ServerFactory(null, null));
 			IAssemblyInformationProvider assemblyInformationProvider = Mock.Of<IAssemblyInformationProvider>();
@@ -31,7 +33,7 @@ public async Task TestWorksWithoutUpdatePath()
 			var factory = Application.CreateDefaultServerFactory();
 
 			await Assert.ThrowsExceptionAsync<ArgumentNullException>(() => factory.CreateServer(null, null, default).AsTask());
-			var result = await factory.CreateServer(new[] { "General:SetupWizardMode=Never" }, null, default);
+			var result = await factory.CreateServer(cliArgs, null, default);
 			Assert.IsNotNull(result);
 		}
 
@@ -43,7 +45,7 @@ public async Task TestWorksWithUpdatePath()
 
 			await Assert.ThrowsExceptionAsync<ArgumentNullException>(() => factory.CreateServer(null, null, default).AsTask());
 			await Assert.ThrowsExceptionAsync<ArgumentNullException>(() => factory.CreateServer(null, Path, default).AsTask());
-			var result = await factory.CreateServer(new[] { "General:SetupWizardMode=Never" }, Path, default);
+			var result = await factory.CreateServer(cliArgs, Path, default);
 			Assert.IsNotNull(result);
 		}
 	}