From e684b76a3c0a7c564f6fef459959d6c4cef7907e Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sat, 2 Mar 2024 14:47:04 -0500 Subject: [PATCH 1/7] Nuget package update --- src/Tgstation.Server.Api/Tgstation.Server.Api.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tgstation.Server.Api/Tgstation.Server.Api.csproj b/src/Tgstation.Server.Api/Tgstation.Server.Api.csproj index 0b93c41fec8..b7e14a249bd 100644 --- a/src/Tgstation.Server.Api/Tgstation.Server.Api.csproj +++ b/src/Tgstation.Server.Api/Tgstation.Server.Api.csproj @@ -27,7 +27,7 @@ - + From bfc2e64e663869105ce6680b6538f9356bbd3b85 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sat, 2 Mar 2024 15:35:58 -0500 Subject: [PATCH 2/7] Set `oom_score_adj` appropriately on Linux Closes #1792 --- README.md | 1 + .../Components/Deployment/DreamMaker.cs | 3 +- .../Components/Engine/OpenDreamInstaller.cs | 3 +- .../Engine/WindowsByondInstaller.cs | 3 +- .../Session/SessionControllerFactory.cs | 3 +- .../Components/StaticFiles/Configuration.cs | 3 +- src/Tgstation.Server.Host/Core/Application.cs | 1 + .../IO/DefaultIOManager.cs | 22 ++-- src/Tgstation.Server.Host/IO/IIOManager.cs | 7 ++ .../System/IProcessExecutor.cs | 8 +- .../System/IProcessFeatures.cs | 14 ++- .../System/PosixProcessFeatures.cs | 104 +++++++++++++++++- .../System/ProcessExecutor.cs | 14 ++- .../System/WindowsFirewallHelper.cs | 3 +- .../System/WindowsProcessFeatures.cs | 4 + .../System/TestPosixSignalHandler.cs | 3 +- .../Live/Instance/WatchdogTest.cs | 4 +- .../Live/TestLiveServer.cs | 3 +- .../TestSystemInteraction.cs | 4 +- tests/Tgstation.Server.Tests/TestVersions.cs | 5 +- 20 files changed, 181 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index 085f8c05dba..f5351741188 100644 --- a/README.md +++ b/README.md @@ -157,6 +157,7 @@ docker run \ --network="host" \ # Not recommended, eases networking setup if your sql server is on the same machine --name="tgs" \ # Name for the container --cap-add=sys_nice \ # Recommended, allows TGS to lower the niceness of child processes if it sees fit + --cap-add=sys_resource \ # Recommended, allows TGS to not be killed by the OOM killer before its child processes --init \ #Highly recommended, reaps potential zombie processes -p 5000:5000 \ # Port bridge for accessing TGS, you can change this if you need -p 0.0.0.0:: \ # Port bridge for accessing DreamDaemon diff --git a/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs b/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs index 408cc07bfbe..27107f7ff85 100644 --- a/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs +++ b/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs @@ -855,11 +855,12 @@ async ValueTask RunDreamMaker(IEngineExecutableLock engineLock, Models.Com var environment = await engineLock.LoadEnv(logger, true, cancellationToken); var arguments = engineLock.FormatCompilerArguments($"{job.DmeName}.{DmeExtension}"); - await using var dm = processExecutor.LaunchProcess( + await using var dm = await processExecutor.LaunchProcess( engineLock.CompilerExePath, ioManager.ResolvePath( job.DirectoryName!.Value.ToString()), arguments, + cancellationToken, environment, readStandardHandles: true, noShellExecute: true); diff --git a/src/Tgstation.Server.Host/Components/Engine/OpenDreamInstaller.cs b/src/Tgstation.Server.Host/Components/Engine/OpenDreamInstaller.cs index eb0bca94504..e552d16b406 100644 --- a/src/Tgstation.Server.Host/Components/Engine/OpenDreamInstaller.cs +++ b/src/Tgstation.Server.Host/Components/Engine/OpenDreamInstaller.cs @@ -241,10 +241,11 @@ await HandleExtremelyLongPathOperation( async shortenedPath => { var shortenedDeployPath = IOManager.ConcatPath(shortenedPath, DeployDir); - await using var buildProcess = ProcessExecutor.LaunchProcess( + await using var buildProcess = await ProcessExecutor.LaunchProcess( dotnetPath, shortenedPath, $"run -c Release --project OpenDreamPackageTool -- --tgs -o {shortenedDeployPath}", + cancellationToken, null, null, !GeneralConfiguration.OpenDreamSuppressInstallOutput, diff --git a/src/Tgstation.Server.Host/Components/Engine/WindowsByondInstaller.cs b/src/Tgstation.Server.Host/Components/Engine/WindowsByondInstaller.cs index 1534bfc5000..35bcbf63dc0 100644 --- a/src/Tgstation.Server.Host/Components/Engine/WindowsByondInstaller.cs +++ b/src/Tgstation.Server.Host/Components/Engine/WindowsByondInstaller.cs @@ -283,10 +283,11 @@ async ValueTask InstallDirectX(string path, CancellationToken cancellationToken) try { // noShellExecute because we aren't doing runas shennanigans - await using var directXInstaller = processExecutor.LaunchProcess( + await using var directXInstaller = await processExecutor.LaunchProcess( IOManager.ConcatPath(rbdx, "DXSETUP.exe"), rbdx, "/silent", + cancellationToken, noShellExecute: true); int exitCode; diff --git a/src/Tgstation.Server.Host/Components/Session/SessionControllerFactory.cs b/src/Tgstation.Server.Host/Components/Session/SessionControllerFactory.cs index 5e89fe8cb24..1573d4c48aa 100644 --- a/src/Tgstation.Server.Host/Components/Session/SessionControllerFactory.cs +++ b/src/Tgstation.Server.Host/Components/Session/SessionControllerFactory.cs @@ -516,10 +516,11 @@ async ValueTask CreateGameServerProcess( ? logFilePath : null); - var process = processExecutor.LaunchProcess( + var process = await processExecutor.LaunchProcess( engineLock.ServerExePath, dmbProvider.Directory, arguments, + cancellationToken, environment, logFilePath, engineLock.HasStandardOutput, diff --git a/src/Tgstation.Server.Host/Components/StaticFiles/Configuration.cs b/src/Tgstation.Server.Host/Components/StaticFiles/Configuration.cs index f25f515a605..86ae2c28ca8 100644 --- a/src/Tgstation.Server.Host/Components/StaticFiles/Configuration.cs +++ b/src/Tgstation.Server.Host/Components/StaticFiles/Configuration.cs @@ -761,7 +761,7 @@ async ValueTask ExecuteEventScripts(IEnumerable parameters, bool deploy foreach (var scriptFile in scriptFiles) { logger.LogTrace("Running event script {scriptFile}...", scriptFile); - await using (var script = processExecutor.LaunchProcess( + await using (var script = await processExecutor.LaunchProcess( ioManager.ConcatPath(resolvedScriptsDir, scriptFile), resolvedScriptsDir, String.Join( @@ -778,6 +778,7 @@ async ValueTask ExecuteEventScripts(IEnumerable parameters, bool deploy return $"\"{arg}\""; })), + cancellationToken, readStandardHandles: true, noShellExecute: true)) using (cancellationToken.Register(() => script.Terminate())) diff --git a/src/Tgstation.Server.Host/Core/Application.cs b/src/Tgstation.Server.Host/Core/Application.cs index bad47cde0be..9569d2510b7 100644 --- a/src/Tgstation.Server.Host/Core/Application.cs +++ b/src/Tgstation.Server.Host/Core/Application.cs @@ -356,6 +356,7 @@ void AddTypedContext() services.AddSingleton(); services.AddSingleton(); + services.AddHostedService(); // PosixProcessFeatures also needs a IProcessExecutor for gcore services.AddSingleton(x => new Lazy(() => x.GetRequiredService(), true)); diff --git a/src/Tgstation.Server.Host/IO/DefaultIOManager.cs b/src/Tgstation.Server.Host/IO/DefaultIOManager.cs index cf7b6165873..0ce1d50f45d 100644 --- a/src/Tgstation.Server.Host/IO/DefaultIOManager.cs +++ b/src/Tgstation.Server.Host/IO/DefaultIOManager.cs @@ -221,14 +221,7 @@ public Task MoveDirectory(string source, string destination, CancellationToken c /// public async ValueTask ReadAllBytes(string path, CancellationToken cancellationToken) { - path = ResolvePath(path); - await using var file = new FileStream( - path, - FileMode.Open, - FileAccess.Read, - FileShare.ReadWrite | FileShare.Delete, - DefaultBufferSize, - FileOptions.Asynchronous | FileOptions.SequentialScan); + await using var file = CreateAsyncSequentialReadStream(path); byte[] buf; buf = new byte[file.Length]; await file.ReadAsync(buf, cancellationToken); @@ -261,6 +254,19 @@ public FileStream CreateAsyncSequentialWriteStream(string path) FileOptions.Asynchronous | FileOptions.SequentialScan); } + /// + public FileStream CreateAsyncSequentialReadStream(string path) + { + path = ResolvePath(path); + return new FileStream( + path, + FileMode.Open, + FileAccess.Read, + FileShare.ReadWrite | FileShare.Delete, + DefaultBufferSize, + FileOptions.Asynchronous | FileOptions.SequentialScan); + } + /// public Task> GetDirectories(string path, CancellationToken cancellationToken) => Task.Factory.StartNew( () => diff --git a/src/Tgstation.Server.Host/IO/IIOManager.cs b/src/Tgstation.Server.Host/IO/IIOManager.cs index d88ec3ec176..8b8b6f0ce79 100644 --- a/src/Tgstation.Server.Host/IO/IIOManager.cs +++ b/src/Tgstation.Server.Host/IO/IIOManager.cs @@ -110,6 +110,13 @@ ValueTask CopyDirectory( /// The open . FileStream CreateAsyncSequentialWriteStream(string path); + /// + /// Creates an asynchronous for sequential reading. + /// + /// The path of the file to write, will be truncated. + /// The open . + FileStream CreateAsyncSequentialReadStream(string path); + /// /// Writes some to a file at overwriting previous content. /// diff --git a/src/Tgstation.Server.Host/System/IProcessExecutor.cs b/src/Tgstation.Server.Host/System/IProcessExecutor.cs index 34962811e14..0ce0c98a8c8 100644 --- a/src/Tgstation.Server.Host/System/IProcessExecutor.cs +++ b/src/Tgstation.Server.Host/System/IProcessExecutor.cs @@ -1,4 +1,6 @@ using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; namespace Tgstation.Server.Host.System { @@ -13,15 +15,17 @@ interface IProcessExecutor /// The full path to the executable file. /// The working directory for the . /// The arguments for the . + /// The for the operation. /// A of environment variables to set. /// File to write process output and error streams to. Requires to be . /// If the process output and error streams should be read. /// If shell execute should not be used. Must be set if is set. - /// The new . - IProcess LaunchProcess( + /// A resulting in the new . + ValueTask LaunchProcess( string fileName, string workingDirectory, string arguments, + CancellationToken cancellationToken, IReadOnlyDictionary? environment = null, string? fileRedirect = null, bool readStandardHandles = false, diff --git a/src/Tgstation.Server.Host/System/IProcessFeatures.cs b/src/Tgstation.Server.Host/System/IProcessFeatures.cs index 927afd7e634..b00c5a49340 100644 --- a/src/Tgstation.Server.Host/System/IProcessFeatures.cs +++ b/src/Tgstation.Server.Host/System/IProcessFeatures.cs @@ -18,11 +18,11 @@ interface IProcessFeatures /// /// Suspend a given . /// - /// The to suspend. + /// The to suspend. void SuspendProcess(global::System.Diagnostics.Process process); /// - /// Resume a given suspended . + /// Resume a given suspended . /// /// The to suspended. void ResumeProcess(global::System.Diagnostics.Process process); @@ -30,11 +30,19 @@ interface IProcessFeatures /// /// Create a dump file for a given . /// - /// The to dump. + /// The to dump. /// The full path to the output file. /// If a minidump should be taken as opposed to a full dump. /// The for the operation. /// A representing the running operation. ValueTask CreateDump(global::System.Diagnostics.Process process, string outputFile, bool minidump, CancellationToken cancellationToken); + + /// + /// Run events on starting a process. + /// + /// The that was started. + /// The for the operation. + /// A resulting in the ID. + ValueTask HandleProcessStart(global::System.Diagnostics.Process process, CancellationToken cancellationToken); } } diff --git a/src/Tgstation.Server.Host/System/PosixProcessFeatures.cs b/src/Tgstation.Server.Host/System/PosixProcessFeatures.cs index 7fd2ce85590..9d97d20be5a 100644 --- a/src/Tgstation.Server.Host/System/PosixProcessFeatures.cs +++ b/src/Tgstation.Server.Host/System/PosixProcessFeatures.cs @@ -1,7 +1,11 @@ using System; +using System.Globalization; +using System.IO; +using System.Text; using System.Threading; using System.Threading.Tasks; +using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; using Mono.Unix; using Mono.Unix.Native; @@ -13,8 +17,18 @@ namespace Tgstation.Server.Host.System { /// - sealed class PosixProcessFeatures : IProcessFeatures + sealed class PosixProcessFeatures : IProcessFeatures, IHostedService { + /// + /// Difference from to set our own oom_score_adj to. 1 higher host watchdog. + /// + const short SelfOomAdjust = 1; + + /// + /// Difference from to set the oom_score_adj of child processes to. 1 higher than ourselves. + /// + const short ChildProcessOomAdjust = SelfOomAdjust + 1; + /// /// loaded . /// @@ -30,6 +44,11 @@ sealed class PosixProcessFeatures : IProcessFeatures /// readonly ILogger logger; + /// + /// The original value of oom_score_adj as read from the /proc/ filesystem. Inherited from parent process. + /// + short baselineOomAdjust; + /// /// Initializes a new instance of the class. /// @@ -88,10 +107,11 @@ public async ValueTask CreateDump(global::System.Diagnostics.Process process, st string? output; int exitCode; - await using (var gcoreProc = lazyLoadedProcessExecutor.Value.LaunchProcess( + await using (var gcoreProc = await lazyLoadedProcessExecutor.Value.LaunchProcess( GCorePath, Environment.CurrentDirectory, $"{(!minidump ? "-a " : String.Empty)}-o {outputFile} {process.Id}", + cancellationToken, readStandardHandles: true, noShellExecute: true)) { @@ -112,5 +132,85 @@ public async ValueTask CreateDump(global::System.Diagnostics.Process process, st var generatedGCoreFile = $"{outputFile}.{pid}"; await ioManager.MoveFile(generatedGCoreFile, outputFile, cancellationToken); } + + /// + public async ValueTask HandleProcessStart(global::System.Diagnostics.Process process, CancellationToken cancellationToken) + { + ArgumentNullException.ThrowIfNull(process); + var pid = process.Id; + try + { + // make sure all processes we spawn are killed _before_ us + await AdjustOutOfMemoryScore(pid, ChildProcessOomAdjust, cancellationToken); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + logger.LogWarning(ex, "Failed to adjust OOM killer score for pid {pid}!", pid); + } + + return pid; + } + + /// + public async Task StartAsync(CancellationToken cancellationToken) + { + // let this all throw + string originalString; + { + // can't use ReadAllBytes here, /proc files have 0 length so the buffer is initialized to empty + // https://stackoverflow.com/questions/12237712/how-can-i-show-the-size-of-files-in-proc-it-should-not-be-size-zero + await using var fileStream = ioManager.CreateAsyncSequentialReadStream( + "/proc/self/oom_score_adj"); + using var reader = new StreamReader(fileStream, Encoding.UTF8, leaveOpen: true); + originalString = await reader.ReadToEndAsync(cancellationToken); + } + + var trimmedString = originalString.Trim(); + + logger.LogTrace("Original oom_score_adj is \"{original}\"", trimmedString); + + var originalOomAdjust = Int16.Parse(trimmedString, CultureInfo.InvariantCulture); + baselineOomAdjust = Math.Clamp(originalOomAdjust, (short)-1000, (short)1000); + + if (originalOomAdjust != baselineOomAdjust) + logger.LogWarning("oom_score_adj is at it's limit of 1000 (Clamped from {original}). TGS cannot guarantee the kill order of its parent/child processes!", originalOomAdjust); + else + logger.LogWarning("oom_score_adj is at it's limit of 1000. TGS cannot guarantee the kill order of its parent/child processes!"); + + try + { + // we do not want to be killed before the host watchdog + await AdjustOutOfMemoryScore(null, SelfOomAdjust, cancellationToken); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + logger.LogWarning(ex, "Could not increase oom_score_adj!"); + } + } + + /// + public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask; + + /// + /// Set oom_score_adj for a given . + /// + /// The or to self adjust. + /// The value being written to the adjustment file. + /// The for the operation. + /// A representing the running operation. + ValueTask AdjustOutOfMemoryScore(int? pid, short adjustment, CancellationToken cancellationToken) + { + var adjustedValue = Math.Clamp(baselineOomAdjust + adjustment, -1000, 1000); + + var pidStr = pid.HasValue + ? pid.Value.ToString(CultureInfo.InvariantCulture) + : "self"; + logger.LogTrace( + "Setting oom_score_adj of {pid} to {adjustment}...", pidStr, adjustedValue); + return ioManager.WriteAllBytes( + $"/proc/{pidStr}/oom_score_adj", + Encoding.UTF8.GetBytes(adjustedValue.ToString(CultureInfo.InvariantCulture)), + cancellationToken); + } } } diff --git a/src/Tgstation.Server.Host/System/ProcessExecutor.cs b/src/Tgstation.Server.Host/System/ProcessExecutor.cs index 445a333ffdf..01fe2c00a48 100644 --- a/src/Tgstation.Server.Host/System/ProcessExecutor.cs +++ b/src/Tgstation.Server.Host/System/ProcessExecutor.cs @@ -105,10 +105,11 @@ public IProcess GetCurrentProcess() } /// - public IProcess LaunchProcess( + public async ValueTask LaunchProcess( string fileName, string workingDirectory, string arguments, + CancellationToken cancellationToken, IReadOnlyDictionary? environment, string? fileRedirect, bool readStandardHandles, @@ -174,7 +175,16 @@ public IProcess LaunchProcess( ExclusiveProcessLaunchLock.ExitReadLock(); } - pid = handle.Id; + try + { + pid = await processFeatures.HandleProcessStart(handle, cancellationToken); + } + catch + { + handle.Kill(); + throw; + } + processStartTcs?.SetResult(pid); } catch (Exception ex) diff --git a/src/Tgstation.Server.Host/System/WindowsFirewallHelper.cs b/src/Tgstation.Server.Host/System/WindowsFirewallHelper.cs index fff15210927..13e1171053e 100644 --- a/src/Tgstation.Server.Host/System/WindowsFirewallHelper.cs +++ b/src/Tgstation.Server.Host/System/WindowsFirewallHelper.cs @@ -31,10 +31,11 @@ public static async ValueTask AddFirewallException( { logger.LogInformation("Adding Windows Firewall exception for {path}...", exePath); var arguments = $"advfirewall firewall add rule name=\"{exceptionName}\" program=\"{exePath}\" protocol=tcp dir=in enable=yes action=allow"; - await using var netshProcess = processExecutor.LaunchProcess( + await using var netshProcess = await processExecutor.LaunchProcess( "netsh.exe", Environment.CurrentDirectory, arguments, + cancellationToken, readStandardHandles: true, noShellExecute: true); diff --git a/src/Tgstation.Server.Host/System/WindowsProcessFeatures.cs b/src/Tgstation.Server.Host/System/WindowsProcessFeatures.cs index f7ad2096a1a..6b71b2b2539 100644 --- a/src/Tgstation.Server.Host/System/WindowsProcessFeatures.cs +++ b/src/Tgstation.Server.Host/System/WindowsProcessFeatures.cs @@ -172,5 +172,9 @@ await Task.Factory.StartNew( DefaultIOManager.BlockingTaskCreationOptions, TaskScheduler.Current); } + + /// + public ValueTask HandleProcessStart(global::System.Diagnostics.Process process, CancellationToken cancellationToken) + => ValueTask.FromResult((process ?? throw new ArgumentNullException(nameof(process))).Id); } } diff --git a/tests/Tgstation.Server.Host.Tests/System/TestPosixSignalHandler.cs b/tests/Tgstation.Server.Host.Tests/System/TestPosixSignalHandler.cs index 121df115e3c..dd0f5fd5fb2 100644 --- a/tests/Tgstation.Server.Host.Tests/System/TestPosixSignalHandler.cs +++ b/tests/Tgstation.Server.Host.Tests/System/TestPosixSignalHandler.cs @@ -63,11 +63,12 @@ public async Task TestSignalListening() Mock.Of(), loggerFactory.CreateLogger(), loggerFactory); - await using var subProc = processExecutor + await using var subProc = await processExecutor .LaunchProcess( "dotnet", pathToSignalTestApp, $"run -c {CurrentConfig} --no-build", + CancellationToken.None, null, null, true, diff --git a/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs b/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs index 6a59da10bbf..48d8a5faee6 100644 --- a/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs +++ b/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs @@ -731,7 +731,7 @@ void TestLinuxIsntBeingFuckingCheekyAboutFilePaths(DreamDaemonResponse currentSt var features = new PosixProcessFeatures( new Lazy(Mock.Of()), - Mock.Of(), + new DefaultIOManager(), Mock.Of>()); features.SuspendProcess(proc); @@ -798,7 +798,7 @@ async Task RunHealthCheckTest(bool checkDump, CancellationToken cancellationToke executor = new ProcessExecutor( RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? new WindowsProcessFeatures(Mock.Of>()) - : new PosixProcessFeatures(new Lazy(() => executor), Mock.Of(), Mock.Of>()), + : new PosixProcessFeatures(new Lazy(() => executor), new DefaultIOManager(), Mock.Of>()), Mock.Of(), Mock.Of>(), LoggerFactory.Create(x => { })); diff --git a/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs b/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs index 5e058d24eb4..b218b146802 100644 --- a/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs +++ b/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs @@ -1100,10 +1100,11 @@ await ioManager.CopyDirectory( async ValueTask RunGitCommand(string args) { - await using var gitRemoteOriginFixProc = processExecutor.LaunchProcess( + await using var gitRemoteOriginFixProc = await processExecutor.LaunchProcess( "git", repoPath, args, + cancellationToken, null, null, true, diff --git a/tests/Tgstation.Server.Tests/TestSystemInteraction.cs b/tests/Tgstation.Server.Tests/TestSystemInteraction.cs index a2b43af6fdc..e7c8e289620 100644 --- a/tests/Tgstation.Server.Tests/TestSystemInteraction.cs +++ b/tests/Tgstation.Server.Tests/TestSystemInteraction.cs @@ -28,7 +28,7 @@ public async Task TestScriptExecutionWithStdRead() Mock.Of>(), loggerFactory); - await using var process = processExecutor.LaunchProcess("test." + platformIdentifier.ScriptFileExtension, ".", string.Empty, null, null, true, true); + await using var process = await processExecutor.LaunchProcess("test." + platformIdentifier.ScriptFileExtension, ".", string.Empty, CancellationToken.None, null, null, true, true); using var cts = new CancellationTokenSource(); cts.CancelAfter(3000); var exitCode = await process.Lifetime.WaitAsync(cts.Token); @@ -63,7 +63,7 @@ public async Task TestScriptExecutionWithFileOutput() File.Delete(tempFile); try { - await using (var process = processExecutor.LaunchProcess("test." + platformIdentifier.ScriptFileExtension, ".", string.Empty, null, tempFile, true, true)) + await using (var process = await processExecutor.LaunchProcess("test." + platformIdentifier.ScriptFileExtension, ".", string.Empty, CancellationToken.None, null, tempFile, true, true)) { using var cts = new CancellationTokenSource(); cts.CancelAfter(3000); diff --git a/tests/Tgstation.Server.Tests/TestVersions.cs b/tests/Tgstation.Server.Tests/TestVersions.cs index 0b4cc591bb3..adadd4bfcb2 100644 --- a/tests/Tgstation.Server.Tests/TestVersions.cs +++ b/tests/Tgstation.Server.Tests/TestVersions.cs @@ -208,7 +208,7 @@ await CachingFileDownloader.InitializeByondVersion( ? new WindowsProcessFeatures(Mock.Of>()) : new PosixProcessFeatures( new Lazy(() => null), - Mock.Of(), + new DefaultIOManager(), loggerFactory.CreateLogger()), Mock.Of(), loggerFactory.CreateLogger(), @@ -498,10 +498,11 @@ static async Task TestMapThreadsVersion( try { - await using var process = processExecutor.LaunchProcess( + await using var process = await processExecutor.LaunchProcess( ddPath, Environment.CurrentDirectory, "fake.dmb -map-threads 3 -close", + CancellationToken.None, null, null, true, From 2b44a592f6e56ad3dc354482e23a2ad8db22a669 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sun, 3 Mar 2024 10:20:50 -0500 Subject: [PATCH 3/7] Attempt to deduce GitHub and GitLab test merge URLs if API calls fail Closes #1795 --- .../Components/Repository/GitHubRemoteFeatures.cs | 2 +- .../Components/Repository/GitLabRemoteFeatures.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Repository/GitHubRemoteFeatures.cs b/src/Tgstation.Server.Host/Components/Repository/GitHubRemoteFeatures.cs index 7a21ebde2b9..6109f490bb7 100644 --- a/src/Tgstation.Server.Host/Components/Repository/GitHubRemoteFeatures.cs +++ b/src/Tgstation.Server.Host/Components/Repository/GitHubRemoteFeatures.cs @@ -93,7 +93,7 @@ public GitHubRemoteFeatures(IGitHubServiceFactory gitHubServiceFactory, ILogger< Comment = parameters.Comment, Number = parameters.Number, TargetCommitSha = revisionToUse, - Url = pr?.HtmlUrl ?? errorMessage, + Url = pr?.HtmlUrl ?? $"https://github.com/{RemoteRepositoryOwner}/{RemoteRepositoryName}/pull/{parameters.Number}", }; return testMerge; diff --git a/src/Tgstation.Server.Host/Components/Repository/GitLabRemoteFeatures.cs b/src/Tgstation.Server.Host/Components/Repository/GitLabRemoteFeatures.cs index 9e468b06e66..9cf32f3f7bd 100644 --- a/src/Tgstation.Server.Host/Components/Repository/GitLabRemoteFeatures.cs +++ b/src/Tgstation.Server.Host/Components/Repository/GitLabRemoteFeatures.cs @@ -85,7 +85,7 @@ public GitLabRemoteFeatures(ILogger logger, Uri remoteUrl) Comment = parameters.Comment, Number = parameters.Number, TargetCommitSha = parameters.TargetCommitSha, - Url = ex.Message, + Url = $"https://gitlab.com/{RemoteRepositoryOwner}/{RemoteRepositoryName}/-/merge_requests/{parameters.Number}", }; } } From ffe273b0c9d00a8792b01567d2eaaa444a9d0df1 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sun, 3 Mar 2024 13:36:18 -0500 Subject: [PATCH 4/7] A note about `IIOManager.ReadAllBytes` and `/proc` --- src/Tgstation.Server.Host/IO/IIOManager.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Tgstation.Server.Host/IO/IIOManager.cs b/src/Tgstation.Server.Host/IO/IIOManager.cs index 8b8b6f0ce79..e14f15cf292 100644 --- a/src/Tgstation.Server.Host/IO/IIOManager.cs +++ b/src/Tgstation.Server.Host/IO/IIOManager.cs @@ -85,6 +85,7 @@ ValueTask CopyDirectory( /// The path of the file to read. /// A for the operation. /// A that results in the contents of a file at . + /// This function will fail to read files from the /proc filesystem on Linux. ValueTask ReadAllBytes(string path, CancellationToken cancellationToken); /// From c4f194a61ed086b1e83398f7a0e5a7795e9355ed Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sun, 3 Mar 2024 16:29:10 -0500 Subject: [PATCH 5/7] Update Master Merge workflow --- .github/workflows/stable-merge.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/stable-merge.yml b/.github/workflows/stable-merge.yml index 9135dc5f7a1..bd4d64d09ff 100644 --- a/.github/workflows/stable-merge.yml +++ b/.github/workflows/stable-merge.yml @@ -17,7 +17,7 @@ jobs: fetch-depth: 0 - name: Merge master into dev - uses: robotology/gh-action-nightly-merge@22f5e45d028f22837d617fa07512925457eec184 #v1.3.3 + uses: robotology/gh-action-nightly-merge@14b4a4cf358f7479aa708bee05cf8a794d6a2516 #v1.5.0 with: stable_branch: 'master' development_branch: 'dev' From e3fe9b5c42fb17429f7cbac2d50cdf08d6ff5c17 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sun, 3 Mar 2024 21:31:05 -0500 Subject: [PATCH 6/7] Version bump to 6.4.0 --- build/Version.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/Version.props b/build/Version.props index bd546a1cd3b..391a9e4ec86 100644 --- a/build/Version.props +++ b/build/Version.props @@ -3,7 +3,7 @@ - 6.3.2 + 6.4.0 5.1.0 10.2.0 7.0.0 From 9b54ea7c4d75a0c7d7930991c6ed41e0362fad71 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sun, 3 Mar 2024 23:53:11 -0500 Subject: [PATCH 7/7] Fix `CI Completion` check not being created for the right SHA --- .github/workflows/ci-pipeline.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci-pipeline.yml b/.github/workflows/ci-pipeline.yml index d136649c083..61a7d50a23b 100644 --- a/.github/workflows/ci-pipeline.yml +++ b/.github/workflows/ci-pipeline.yml @@ -1420,7 +1420,12 @@ jobs: - name: Build ReleaseNotes run: dotnet build -c Release -p:TGS_HOST_NO_WEBPANEL=true tools/Tgstation.Server.ReleaseNotes/Tgstation.Server.ReleaseNotes.csproj - - name: Run ReleaseNotes Create CI Completion Check + - name: Run ReleaseNotes Create CI Completion Check (PR HEAD) + if: github.event_name != 'push' && github.event_name != 'schedule' + run: dotnet run -c Release --no-build --project tools/Tgstation.Server.ReleaseNotes --ci-completion-check ${{ github.event.pull_request.head.sha }} ${{ secrets.TGS_CI_GITHUB_APP_TOKEN_BASE64 }} + + - name: Run ReleaseNotes Create CI Completion Check (Branch) + if: github.event_name == 'push' || github.event_name == 'schedule' run: dotnet run -c Release --no-build --project tools/Tgstation.Server.ReleaseNotes --ci-completion-check ${{ github.sha }} ${{ secrets.TGS_CI_GITHUB_APP_TOKEN_BASE64 }} deployment-gate: