From f374921278c4c2c7a2f40634926e6edfda4a1de4 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Fri, 5 Jan 2024 10:27:30 -0500 Subject: [PATCH 1/3] Log warnings as to why ports cannot be allocated. Also fix raciness in `PortAllocator`. --- src/Tgstation.Server.Host/Core/Application.cs | 2 +- .../Utils/PortAllocator.cs | 81 ++++++++++++++++--- 2 files changed, 71 insertions(+), 12 deletions(-) diff --git a/src/Tgstation.Server.Host/Core/Application.cs b/src/Tgstation.Server.Host/Core/Application.cs index 7e16addfe8..702ab054da 100644 --- a/src/Tgstation.Server.Host/Core/Application.cs +++ b/src/Tgstation.Server.Host/Core/Application.cs @@ -405,7 +405,7 @@ namespace Tgstation.Server.Host.Core services.AddSingleton(x => x.GetRequiredService()); // configure component services - services.AddScoped(); + services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); diff --git a/src/Tgstation.Server.Host/Utils/PortAllocator.cs b/src/Tgstation.Server.Host/Utils/PortAllocator.cs index 1aa096ab0b..ecaf79eeda 100644 --- a/src/Tgstation.Server.Host/Utils/PortAllocator.cs +++ b/src/Tgstation.Server.Host/Utils/PortAllocator.cs @@ -17,7 +17,7 @@ using Tgstation.Server.Host.System; namespace Tgstation.Server.Host.Utils { /// - sealed class PortAllocator : IPortAllocator + sealed class PortAllocator : IPortAllocator, IDisposable { /// /// The for the . @@ -27,7 +27,7 @@ namespace Tgstation.Server.Host.Utils /// /// The for the . /// - readonly IDatabaseContext databaseContext; + readonly IDatabaseContextFactory databaseContextFactory; /// /// The for the . @@ -44,45 +44,79 @@ namespace Tgstation.Server.Host.Utils /// readonly SwarmConfiguration swarmConfiguration; + /// + /// The used to serialized port requisition requests. + /// + readonly SemaphoreSlim allocatorLock; + /// /// Initializes a new instance of the class. /// /// The value of . - /// The value of . + /// The value of . /// The value of . /// The containing the value of . /// The value of . public PortAllocator( IServerPortProvider serverPortProvider, - IDatabaseContext databaseContext, + IDatabaseContextFactory databaseContextFactory, IPlatformIdentifier platformIdentifier, IOptions swarmConfigurationOptions, ILogger logger) { this.serverPortProvider = serverPortProvider ?? throw new ArgumentNullException(nameof(serverPortProvider)); - this.databaseContext = databaseContext ?? throw new ArgumentNullException(nameof(databaseContext)); + this.databaseContextFactory = databaseContextFactory ?? throw new ArgumentNullException(nameof(databaseContextFactory)); this.platformIdentifier = platformIdentifier ?? throw new ArgumentNullException(nameof(platformIdentifier)); swarmConfiguration = swarmConfigurationOptions?.Value ?? throw new ArgumentNullException(nameof(swarmConfigurationOptions)); this.logger = logger ?? throw new ArgumentNullException(nameof(logger)); + + allocatorLock = new SemaphoreSlim(1); } + /// + public void Dispose() => allocatorLock.Dispose(); + /// public async ValueTask GetAvailablePort(ushort basePort, bool checkOne, CancellationToken cancellationToken) { - logger.LogTrace("Port allocation >= {basePort} requested...", basePort); + ushort? result = null; + using (await SemaphoreSlimContext.Lock(allocatorLock, cancellationToken)) + await databaseContextFactory.UseContext( + async databaseContext => result = await GetAvailablePort(databaseContext, basePort, checkOne, cancellationToken)); + return result; + } + /// + /// Gets a port not currently in use by TGS. + /// + /// The to use. + /// The port to check first. Will not allocate a port lower than this. + /// If only should be checked and no others. + /// The for the operation. + /// A resulting in the first available port on success, on failure. + async ValueTask GetAvailablePort(IDatabaseContext databaseContext, ushort basePort, bool checkOne, CancellationToken cancellationToken) + { + logger.LogTrace("Port allocation >= {basePort} requested...", basePort); var ddPorts = await databaseContext .DreamDaemonSettings .AsQueryable() .Where(x => x.Instance!.SwarmIdentifer == swarmConfiguration.Identifier) - .Select(x => x.Port) + .Select(x => new + { + Port = x.Port!.Value, + x.InstanceId, + }) .ToListAsync(cancellationToken); var dmPorts = await databaseContext .DreamMakerSettings .AsQueryable() .Where(x => x.Instance!.SwarmIdentifer == swarmConfiguration.Identifier) - .Select(x => x.ApiValidationPort) + .Select(x => new + { + ApiValidationPort = x.ApiValidationPort!.Value, + x.InstanceId, + }) .ToListAsync(cancellationToken); var exceptions = new List(); @@ -94,10 +128,35 @@ namespace Tgstation.Server.Host.Utils if (checkOne && port != basePort) break; - if (port == serverPortProvider.HttpApiPort - || ddPorts.Contains(port) - || dmPorts.Contains(port)) + if (port == serverPortProvider.HttpApiPort) + { + logger.LogWarning("Cannot allocate port {port} as it is the TGS API port!", port); continue; + } + + var reservedGamePortData = ddPorts.Where(data => data.Port == port).ToList(); + if (reservedGamePortData.Count > 0) + { + logger.LogWarning( + "Cannot allocate port {port} as it in use by the game server of instance(s): {instanceId}!", + port, + String.Join( + ", ", + reservedGamePortData.Select(data => data.InstanceId))); + continue; + } + + var reservedApiValidationPortData = dmPorts.Where(data => data.ApiValidationPort == port).ToList(); + if (reservedApiValidationPortData.Count > 0) + { + logger.LogWarning( + "Cannot allocate port {port} as it in use by the API validation server of instance(s): {instanceId}!", + port, + String.Join( + ", ", + reservedApiValidationPortData.Select(data => data.InstanceId))); + continue; + } try { From efb73f504d5080ed2e252b940a061b0f96710a93 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sat, 6 Jan 2024 17:05:41 -0500 Subject: [PATCH 2/3] Fix issues with spaces in .dme names --- .github/workflows/ci-pipeline.yml | 4 ++-- src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs | 2 +- .../Components/Engine/ByondInstallation.cs | 2 +- .../{basic_operation_test.dme => basic operation_test.dme} | 0 tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs | 4 ++-- tgstation-server.sln | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) rename tests/DMAPI/BasicOperation/{basic_operation_test.dme => basic operation_test.dme} (100%) diff --git a/.github/workflows/ci-pipeline.yml b/.github/workflows/ci-pipeline.yml index bad76f3ba3..cc033d61a7 100644 --- a/.github/workflows/ci-pipeline.yml +++ b/.github/workflows/ci-pipeline.yml @@ -151,7 +151,7 @@ jobs: if hash DreamMaker 2>/dev/null then - DreamMaker tests/DMAPI/BasicOperation/basic_operation_test.dme 2>&1 | tee result.log + DreamMaker "tests/DMAPI/BasicOperation/basic operation_test.dme" 2>&1 | tee result.log retval=$? if ! grep '\- 0 errors, 0 warnings' result.log then @@ -205,7 +205,7 @@ jobs: - name: Build DMAPI run: | cd tests/DMAPI/BasicOperation - $HOME/OpenDream/tgs_deploy/bin/compiler/DMCompiler --verbose --notices-enabled basic_operation_test.dme + $HOME/OpenDream/tgs_deploy/bin/compiler/DMCompiler --verbose --notices-enabled "basic operation_test.dme" pages-build: name: Build gh-pages diff --git a/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs b/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs index f72d848082..adfc780bfa 100644 --- a/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs +++ b/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs @@ -610,7 +610,7 @@ namespace Tgstation.Server.Host.Components.Deployment throw new JobException(ErrorCode.DeploymentMissingDme); } - logger.LogDebug("Selected {dmeName}.dme for compilation!", job.DmeName); + logger.LogDebug("Selected \"{dmeName}.dme\" for compilation!", job.DmeName); progressReporter.StageName = "Modifying .dme"; await ModifyDme(job, cancellationToken); diff --git a/src/Tgstation.Server.Host/Components/Engine/ByondInstallation.cs b/src/Tgstation.Server.Host/Components/Engine/ByondInstallation.cs index fce9b2c5bc..ecfb965e58 100644 --- a/src/Tgstation.Server.Host/Components/Engine/ByondInstallation.cs +++ b/src/Tgstation.Server.Host/Components/Engine/ByondInstallation.cs @@ -118,7 +118,7 @@ namespace Tgstation.Server.Host.Components.Engine var arguments = String.Format( CultureInfo.InvariantCulture, - "{0} -port {1} -ports 1-65535 {2}-close -verbose -{3} -{4}{5}{6}{7} -params \"{8}\"", + "\"{0}\" -port {1} -ports 1-65535 {2}-close -verbose -{3} -{4}{5}{6}{7} -params \"{8}\"", dmbProvider.DmbName, launchParameters.Port!.Value, launchParameters.AllowWebClient!.Value diff --git a/tests/DMAPI/BasicOperation/basic_operation_test.dme b/tests/DMAPI/BasicOperation/basic operation_test.dme similarity index 100% rename from tests/DMAPI/BasicOperation/basic_operation_test.dme rename to tests/DMAPI/BasicOperation/basic operation_test.dme diff --git a/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs b/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs index 87a04a297d..5b18b2417e 100644 --- a/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs +++ b/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs @@ -606,7 +606,7 @@ namespace Tgstation.Server.Tests.Live.Instance var initialCompileJob = daemonStatus.ActiveCompileJob; await CheckDMApiFail(daemonStatus.ActiveCompileJob, cancellationToken); - daemonStatus = await DeployTestDme("BasicOperation/basic_operation_test", DreamDaemonSecurity.Trusted, true, cancellationToken); + daemonStatus = await DeployTestDme("BasicOperation/basic operation_test", DreamDaemonSecurity.Trusted, true, cancellationToken); Assert.AreEqual(WatchdogStatus.Online, daemonStatus.Status.Value); Assert.AreEqual(false, daemonStatus.SoftRestart); // dme name change triggered, instant reboot @@ -629,7 +629,7 @@ namespace Tgstation.Server.Tests.Live.Instance AdditionalParameters = "test=bababooey" }, cancellationToken); Assert.AreEqual("test=bababooey", daemonStatus.AdditionalParameters); - daemonStatus = await DeployTestDme("BasicOperation/basic_operation_test", DreamDaemonSecurity.Trusted, true, cancellationToken); + daemonStatus = await DeployTestDme("BasicOperation/basic operation_test", DreamDaemonSecurity.Trusted, true, cancellationToken); Assert.AreEqual(WatchdogStatus.Offline, daemonStatus.Status.Value); Assert.IsNotNull(daemonStatus.ActiveCompileJob); diff --git a/tgstation-server.sln b/tgstation-server.sln index ce57c2ea30..581410a016 100644 --- a/tgstation-server.sln +++ b/tgstation-server.sln @@ -171,8 +171,8 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "ApiFree", "ApiFree", "{7B8F EndProject Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "BasicOperation", "BasicOperation", "{F32B9514-AAD9-429D-841A-ED810FC2598C}" ProjectSection(SolutionItems) = preProject - tests\DMAPI\BasicOperation\basic_operation_test.dme = tests\DMAPI\BasicOperation\basic_operation_test.dme tests\DMAPI\BasicOperation\Config.dm = tests\DMAPI\BasicOperation\Config.dm + tests\DMAPI\BasicOperation\basic operation_test.dme = tests\DMAPI\BasicOperation\basic operation_test.dme tests\DMAPI\BasicOperation\Test.dm = tests\DMAPI\BasicOperation\Test.dm EndProjectSection EndProject From 86dab9315608474ccd6cd1954c110fda4bd28a7c Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sat, 6 Jan 2024 17:05:54 -0500 Subject: [PATCH 3/3] Version bump to 6.1.2 --- build/Version.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/Version.props b/build/Version.props index d455943bbd..da62ceb40a 100644 --- a/build/Version.props +++ b/build/Version.props @@ -3,7 +3,7 @@ - 6.1.1 + 6.1.2 5.0.0 10.0.0 7.0.0