From 59ffa0838971ddb2266cd4c484864f3fc680f79f Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sat, 2 Mar 2024 13:48:04 -0500 Subject: [PATCH 01/11] Better FromCompileJob lock logging --- .../Components/Deployment/DmbFactory.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Deployment/DmbFactory.cs b/src/Tgstation.Server.Host/Components/Deployment/DmbFactory.cs index d61b2ffc68..92000a1e53 100644 --- a/src/Tgstation.Server.Host/Components/Deployment/DmbFactory.cs +++ b/src/Tgstation.Server.Host/Components/Deployment/DmbFactory.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.IO; @@ -325,14 +325,17 @@ void CleanupAction() if (!jobLockCounts.TryGetValue(compileJobId, out int value)) { value = 1; + logger.LogTrace("Initializing lock count for compile job {id}", compileJobId); jobLockCounts.Add(compileJobId, 1); } else + { + logger.LogTrace("FromCompileJob already had a jobLockCounts entry for {id}. Incrementing lock count to {value}.", compileJobId, value); jobLockCounts[compileJobId] = ++value; + } providerSubmitted = true; - logger.LogTrace("Compile job {id} lock count now: {lockCount}", compileJobId, value); return newProvider; } } From f29ad29bc449373bc96e49243fab061801e4e233 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sat, 2 Mar 2024 13:48:47 -0500 Subject: [PATCH 02/11] Cleanup an unnecessary `catch` --- .../Components/Deployment/DmbFactory.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Deployment/DmbFactory.cs b/src/Tgstation.Server.Host/Components/Deployment/DmbFactory.cs index 92000a1e53..a4ca0c26ca 100644 --- a/src/Tgstation.Server.Host/Components/Deployment/DmbFactory.cs +++ b/src/Tgstation.Server.Host/Components/Deployment/DmbFactory.cs @@ -399,11 +399,7 @@ public async ValueTask CleanUnusedCompileJobs(CancellationToken cancellationToke ++deleting; await DeleteCompileJobContent(x, cancellationToken); } - catch (OperationCanceledException) - { - throw; - } - catch (Exception e) + catch (Exception e) when (e is not OperationCanceledException) { logger.LogWarning(e, "Error deleting directory {dirName}!", x); } From cfc551c547675a19007130bc3b6cf44386cdec5d Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sat, 2 Mar 2024 13:49:06 -0500 Subject: [PATCH 03/11] Switch a lambda to `ValueTask`s --- src/Tgstation.Server.Host/Components/Deployment/DmbFactory.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Deployment/DmbFactory.cs b/src/Tgstation.Server.Host/Components/Deployment/DmbFactory.cs index a4ca0c26ca..ccf40188f8 100644 --- a/src/Tgstation.Server.Host/Components/Deployment/DmbFactory.cs +++ b/src/Tgstation.Server.Host/Components/Deployment/DmbFactory.cs @@ -388,7 +388,7 @@ public async ValueTask CleanUnusedCompileJobs(CancellationToken cancellationToke await ioManager.CreateDirectory(gameDirectory, cancellationToken); var directories = await ioManager.GetDirectories(gameDirectory, cancellationToken); int deleting = 0; - var tasks = directories.Select(async x => + var tasks = directories.Select(async x => { var nameOnly = ioManager.GetFileName(x); if (jobUidsToNotErase.Contains(nameOnly)) @@ -405,7 +405,7 @@ public async ValueTask CleanUnusedCompileJobs(CancellationToken cancellationToke } }).ToList(); if (deleting > 0) - await Task.WhenAll(tasks); + await ValueTaskExtensions.WhenAll(tasks); } #pragma warning restore CA1506 From 94106a3acf5a514ff142239949efbed94aba69b9 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sat, 2 Mar 2024 13:51:43 -0500 Subject: [PATCH 04/11] Better `CompileJob` lock logging --- .../Components/Deployment/DmbFactory.cs | 42 ++++++++++++++++--- 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Deployment/DmbFactory.cs b/src/Tgstation.Server.Host/Components/Deployment/DmbFactory.cs index ccf40188f8..fbeb7d3f92 100644 --- a/src/Tgstation.Server.Host/Components/Deployment/DmbFactory.cs +++ b/src/Tgstation.Server.Host/Components/Deployment/DmbFactory.cs @@ -1,8 +1,9 @@ -using System; +using System; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.IO; using System.Linq; +using System.Text; using System.Threading; using System.Threading.Tasks; @@ -178,7 +179,8 @@ public IDmbProvider LockNextDmb(int lockCount) { var jobId = nextDmbProvider.CompileJob.Require(x => x.Id); var incremented = jobLockCounts[jobId] += lockCount; - logger.LogTrace("Compile job {jobId} lock count now: {lockCount}", jobId, incremented); + logger.LogTrace("Compile job {jobId} lock increased by: {increment}", jobId, lockCount); + LogLockCounts(); return nextDmbProvider; } } @@ -336,6 +338,7 @@ void CleanupAction() providerSubmitted = true; + LogLockCounts(); return newProvider; } } @@ -434,14 +437,14 @@ async Task WrapThrowableTasks() // First kill the GitHub deployment var remoteDeploymentManager = remoteDeploymentManagerFactory.CreateRemoteDeploymentManager(metadata, job); - // DCT: None available - var deploymentJob = remoteDeploymentManager.MarkInactive(job, CancellationToken.None); + var cancellationToken = cleanupCts.Token; + var deploymentJob = remoteDeploymentManager.MarkInactive(job, cancellationToken); - var deleteTask = DeleteCompileJobContent(job.DirectoryName!.Value.ToString(), cleanupCts.Token); + var deleteTask = DeleteCompileJobContent(job.DirectoryName!.Value.ToString(), cancellationToken); await ValueTaskExtensions.WhenAll(deleteTask, deploymentJob); } - catch (Exception ex) + catch (Exception ex) when (ex is not OperationCanceledException) { logger.LogWarning(ex, "Error cleaning up compile job {jobGuid}!", job.DirectoryName); } @@ -467,6 +470,8 @@ async Task WrapThrowableTasks() } else logger.LogError("Extra Dispose of DmbProvider for CompileJob {compileJobId}!", jobId); + + LogLockCounts(); } } @@ -482,5 +487,30 @@ async ValueTask DeleteCompileJobContent(string directory, CancellationToken canc await eventConsumer.HandleEvent(EventType.DeploymentCleanup, new List { ioManager.ResolvePath(directory) }, true, cancellationToken); await ioManager.DeleteDirectory(directory, cancellationToken); } + + /// + /// Log out the current lock counts to Trace. + /// + /// must be locked before calling this function. + void LogLockCounts() + { + if (jobLockCounts.Count == 0) + { + logger.LogWarning("No compile jobs registered!"); + return; + } + + var builder = new StringBuilder(); + foreach (var jobId in jobLockCounts.Keys) + { + builder.AppendLine(); + builder.Append("\t- "); + builder.Append(jobId); + builder.Append(": "); + builder.Append(jobLockCounts[jobId]); + } + + logger.LogTrace("Compile Job Lock Counts:{details}", builder.ToString()); + } } } From c9c6e50284db719805e850e0fab3ede0309b65a8 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sat, 2 Mar 2024 13:52:25 -0500 Subject: [PATCH 05/11] Remove a redundant `await` --- .../Components/Watchdog/AdvancedWatchdog.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Watchdog/AdvancedWatchdog.cs b/src/Tgstation.Server.Host/Components/Watchdog/AdvancedWatchdog.cs index 884541e9fb..aad9bcf64e 100644 --- a/src/Tgstation.Server.Host/Components/Watchdog/AdvancedWatchdog.cs +++ b/src/Tgstation.Server.Host/Components/Watchdog/AdvancedWatchdog.cs @@ -127,8 +127,12 @@ protected sealed override async ValueTask DisposeAndNullControllersImpl() // If we reach this point, we can guarantee PrepServerForLaunch will be called before starting again. ActiveSwappable = null; - await (pendingSwappable?.DisposeAsync() ?? ValueTask.CompletedTask); - pendingSwappable = null; + + if (pendingSwappable != null) + { + await pendingSwappable.DisposeAsync(); + pendingSwappable = null; + } await DrainDeploymentCleanupTasks(true); } From 1beb53fe704571971870a7d91a14e5a29d66c71f Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sat, 2 Mar 2024 13:57:47 -0500 Subject: [PATCH 06/11] Fix double call to `BeforeApplyDmb` --- .../Components/Watchdog/AdvancedWatchdog.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Watchdog/AdvancedWatchdog.cs b/src/Tgstation.Server.Host/Components/Watchdog/AdvancedWatchdog.cs index aad9bcf64e..0ff64a54e4 100644 --- a/src/Tgstation.Server.Host/Components/Watchdog/AdvancedWatchdog.cs +++ b/src/Tgstation.Server.Host/Components/Watchdog/AdvancedWatchdog.cs @@ -142,8 +142,6 @@ protected sealed override async ValueTask HandleNormalReboot(Canc { if (pendingSwappable != null) { - ValueTask RunPrequel() => BeforeApplyDmb(pendingSwappable.CompileJob, cancellationToken); - var needToSwap = !pendingSwappable.Swapped; var controller = Server!; if (needToSwap) @@ -155,7 +153,6 @@ protected sealed override async ValueTask HandleNormalReboot(Canc // integration test logging will catch this Logger.LogError( "The reboot bridge request completed before the watchdog could suspend the server! This can lead to buggy DreamDaemon behaviour and should be reported! To ensure stability, we will need to hard reboot the server"); - await RunPrequel(); return MonitorAction.Restart; } @@ -169,7 +166,7 @@ protected sealed override async ValueTask HandleNormalReboot(Canc } } - var updateTask = RunPrequel(); + var updateTask = BeforeApplyDmb(pendingSwappable.CompileJob, cancellationToken); if (needToSwap) await PerformDmbSwap(pendingSwappable, cancellationToken); From 81dd33c1d1d6fec6b0df2707af74737f9f008ea4 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sat, 2 Mar 2024 13:58:43 -0500 Subject: [PATCH 07/11] Documentation comment cleanup --- .../Components/Deployment/SwappableDmbProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tgstation.Server.Host/Components/Deployment/SwappableDmbProvider.cs b/src/Tgstation.Server.Host/Components/Deployment/SwappableDmbProvider.cs index 75c5a1b6bb..7ad999856c 100644 --- a/src/Tgstation.Server.Host/Components/Deployment/SwappableDmbProvider.cs +++ b/src/Tgstation.Server.Host/Components/Deployment/SwappableDmbProvider.cs @@ -87,7 +87,7 @@ public ValueTask MakeActive(CancellationToken cancellationToken) } /// - /// Should be . before calling to ensure the is ready to instantly swap. Can be called multiple times. + /// Should be ed. before calling to ensure the is ready to instantly swap. Can be called multiple times. /// /// The for the operation. /// A representing the preparation process. From 8b9f5535ca72c63c3e62c441b66911e0a176452d Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sat, 2 Mar 2024 14:17:49 -0500 Subject: [PATCH 08/11] Fix potential for a deployment to leak when stopping or restarting the server I'll say this fixes #1779, but we'll have to see --- .../Components/Watchdog/AdvancedWatchdog.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Tgstation.Server.Host/Components/Watchdog/AdvancedWatchdog.cs b/src/Tgstation.Server.Host/Components/Watchdog/AdvancedWatchdog.cs index 0ff64a54e4..610fe25bba 100644 --- a/src/Tgstation.Server.Host/Components/Watchdog/AdvancedWatchdog.cs +++ b/src/Tgstation.Server.Host/Components/Watchdog/AdvancedWatchdog.cs @@ -184,7 +184,8 @@ async Task CleanupLingeringDeployment() currentCompileJobId, lingeringDeploymentExpirySeconds); - var timeout = AsyncDelayer.Delay(TimeSpan.FromSeconds(lingeringDeploymentExpirySeconds), cancellationToken); + // DCT: A cancel firing here can result in us leaving a dmbprovider undisposed, localDeploymentCleanupGate will always fire in that case + var timeout = AsyncDelayer.Delay(TimeSpan.FromSeconds(lingeringDeploymentExpirySeconds), CancellationToken.None); var completedTask = await Task.WhenAny( localDeploymentCleanupGate.Task, From 5481ebbb7ebc3a7da82466214f52176a35edca22 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sat, 2 Mar 2024 14:18:06 -0500 Subject: [PATCH 09/11] Version bump to 6.3.2 --- build/Version.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/Version.props b/build/Version.props index 34d28b5b29..bd546a1cd3 100644 --- a/build/Version.props +++ b/build/Version.props @@ -3,7 +3,7 @@ - 6.3.1 + 6.3.2 5.1.0 10.2.0 7.0.0 From 4c59fadcfff1b2bb9c43fee44fe75b41e3cf8829 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sat, 2 Mar 2024 14:20:24 -0500 Subject: [PATCH 10/11] Nuget patches --- build/TestCommon.props | 6 +++--- src/Tgstation.Server.Host/Tgstation.Server.Host.csproj | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/build/TestCommon.props b/build/TestCommon.props index af63625813..add8e10ac6 100644 --- a/build/TestCommon.props +++ b/build/TestCommon.props @@ -3,7 +3,7 @@ - + all runtime; build; native; contentfiles; analyzers; buildtransitive @@ -18,9 +18,9 @@ - + - + diff --git a/src/Tgstation.Server.Host/Tgstation.Server.Host.csproj b/src/Tgstation.Server.Host/Tgstation.Server.Host.csproj index 0a689676b5..04df231d71 100644 --- a/src/Tgstation.Server.Host/Tgstation.Server.Host.csproj +++ b/src/Tgstation.Server.Host/Tgstation.Server.Host.csproj @@ -104,7 +104,7 @@ - + @@ -128,7 +128,7 @@ - + From 660b5490cf93b259b423390a5ed7b72a5321ac67 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sat, 2 Mar 2024 17:38:06 -0500 Subject: [PATCH 11/11] Use our own app to create the `CI Completion` check --- .github/workflows/ci-pipeline.yml | 35 ++++++---- .../Tgstation.Server.ReleaseNotes/Program.cs | 64 ++++++++++++++++++- 2 files changed, 87 insertions(+), 12 deletions(-) diff --git a/.github/workflows/ci-pipeline.yml b/.github/workflows/ci-pipeline.yml index e4f02108a3..d136649c08 100644 --- a/.github/workflows/ci-pipeline.yml +++ b/.github/workflows/ci-pipeline.yml @@ -1396,19 +1396,32 @@ jobs: name: CI Completion Gate needs: [ pages-build, docker-build, build-deb, build-msi, validate-openapi-spec, upload-code-coverage, check-winget-pr-template, code-scanning ] runs-on: ubuntu-latest - permissions: - checks: write - contents: read if: (!(cancelled() || failure()) && needs.pages-build.result == 'success' && needs.docker-build.result == 'success' && needs.build-deb.result == 'success' && needs.build-msi.result == 'success' && needs.validate-openapi-spec.result == 'success' && needs.upload-code-coverage.result == 'success' && needs.check-winget-pr-template.result == 'success' && needs.code-scanning.result == 'success') steps: - - name: Create Completion Check - uses: LouisBrunner/checks-action@6b626ffbad7cc56fd58627f774b9067e6118af23 - with: - token: ${{ secrets.GITHUB_TOKEN }} - name: CI Completion - conclusion: success - output: | - {"summary":"The CI Pipeline completed successfully"} + - name: Setup dotnet + uses: actions/setup-dotnet@v4 + with: + dotnet-version: '${{ env.TGS_DOTNET_VERSION }}.0.x' + dotnet-quality: ${{ env.TGS_DOTNET_QUALITY }} + + - name: Checkout (Branch) + uses: actions/checkout@v4 + if: github.event_name == 'push' || github.event_name == 'schedule' + + - name: Checkout (PR Merge) + uses: actions/checkout@v4 + if: github.event_name != 'push' && github.event_name != 'schedule' + with: + ref: "refs/pull/${{ github.event.number }}/merge" + + - name: Restore + run: dotnet restore + + - 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 + 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: name: Deployment Start Gate diff --git a/tools/Tgstation.Server.ReleaseNotes/Program.cs b/tools/Tgstation.Server.ReleaseNotes/Program.cs index 6130bbc17a..d5ac69e564 100644 --- a/tools/Tgstation.Server.ReleaseNotes/Program.cs +++ b/tools/Tgstation.Server.ReleaseNotes/Program.cs @@ -1,20 +1,25 @@ // This program is minimal effort and should be sent to remedial school using System; +using System.Buffers.Text; using System.Collections.Concurrent; using System.Collections.Generic; using System.Diagnostics; +using System.IdentityModel.Tokens.Jwt; using System.IO; using System.Linq; using System.Net; using System.Net.Http; using System.Net.Sockets; using System.Security; +using System.Security.Cryptography; using System.Text; using System.Text.RegularExpressions; using System.Threading.Tasks; using System.Xml.Linq; +using Microsoft.IdentityModel.Tokens; + using Newtonsoft.Json; using Octokit; @@ -32,8 +37,11 @@ namespace Tgstation.Server.ReleaseNotes static class Program { const string OutputPath = "release_notes.md"; + + // some stuff that should be abstracted for different repos const string RepoOwner = "tgstation"; const string RepoName = "tgstation-server"; + const int AppId = 847638; /// /// The entrypoint for the @@ -52,13 +60,15 @@ static async Task Main(string[] args) var shaCheck = versionString.Equals("--winget-template-check", StringComparison.OrdinalIgnoreCase); var fullNotes = versionString.Equals("--generate-full-notes", StringComparison.OrdinalIgnoreCase); var nuget = versionString.Equals("--nuget", StringComparison.OrdinalIgnoreCase); + var ciCompletionCheck = versionString.Equals("--ci-completion-check", StringComparison.OrdinalIgnoreCase); if ((!Version.TryParse(versionString, out var version) || version.Revision != -1) && !ensureRelease && !linkWinget && !shaCheck && !fullNotes - && !nuget) + && !nuget + && !ciCompletionCheck) { Console.WriteLine("Invalid version: " + versionString); return 2; @@ -129,6 +139,17 @@ static async Task Main(string[] args) return await Winget(client, actionsUrl, null); } + if (ciCompletionCheck) + { + if (args.Length < 3) + { + Console.WriteLine("Missing SHA or PEM Base64 for creating check run!"); + return 4543; + } + + return await CICompletionCheck(client, args[1], args[2]); + } + if (shaCheck) { if(args.Length < 2) @@ -1583,6 +1604,47 @@ static async Task GenDebianChangelog(IGitHubClient client, Version version, return 0; } + static async ValueTask CICompletionCheck(GitHubClient gitHubClient, string currentSha, string pemBase64) + { + var pemBytes = Convert.FromBase64String(pemBase64); + var pem = Encoding.UTF8.GetString(pemBytes); + + var rsa = RSA.Create(); + rsa.ImportFromPem(pem); + + var signingCredentials = new SigningCredentials(new RsaSecurityKey(rsa), SecurityAlgorithms.RsaSha256); + var jwtSecurityTokenHandler = new JwtSecurityTokenHandler { SetDefaultTimesOnTokenCreation = false }; + + var now = DateTime.UtcNow; + + var jwt = jwtSecurityTokenHandler.CreateToken(new SecurityTokenDescriptor + { + Issuer = AppId.ToString(), + Expires = now.AddMinutes(10), + IssuedAt = now, + SigningCredentials = signingCredentials + }); + + var jwtStr = jwtSecurityTokenHandler.WriteToken(jwt); + + gitHubClient.Credentials = new Credentials(jwtStr, AuthenticationType.Bearer); + + var installation = await gitHubClient.GitHubApps.GetRepositoryInstallationForCurrent(RepoOwner, RepoName); + var installToken = await gitHubClient.GitHubApps.CreateInstallationToken(installation.Id); + + gitHubClient.Credentials = new Credentials(installToken.Token); + + await gitHubClient.Check.Run.Create(RepoOwner, RepoName, new NewCheckRun("CI Completion", currentSha) + { + CompletedAt = now, + Conclusion = CheckConclusion.Success, + Output = new NewCheckRunOutput("CI Completion", "The CI Pipeline completed successfully"), + Status = CheckStatus.Completed, + }); + + return 0; + } + static void DebugAssert(bool condition, string message = null) { // This exists because one of the fucking asserts evaluates an enumerable or something and it was getting optimized out in release