Merge pull request #1946 from tgstation/MillionthCIFix

Gorillionth CI Fix
tgstation · Oct 3, 2024 · 74860d7 · 74860d7
2 parents fff6410 + 9d2b1d5
commit 74860d7
Show file tree

Hide file tree

Showing 6 changed files with 104 additions and 33 deletions.
diff --git a/.github/workflows/ci-security.yml b/.github/workflows/ci-security.yml
@@ -32,20 +32,6 @@ jobs:
           app-id: ${{ secrets.APP_ID }}
           private-key: ${{ secrets.APP_PRIVATE_KEY }}
 
-      - name: Comment on new Fork PR
-        if: github.event.action == 'opened' && !contains(github.event.pull_request.labels.*.name, 'CI Cleared') && github.event.pull_request.user.id != 49699333
-        uses: thollander/actions-comment-pull-request@fabd468d3a1a0b97feee5f6b9e499eab0dd903f6
-        with:
-          message: Thank you for contributing to ${{ github.event.pull_request.base.repo.name }}! The workflow '${{ github.workflow }}' requires repository secrets and will not run without approval. Maintainers can add the `CI Cleared` label to allow it to run. Note that any changes to ci-security.yml and ci-pipeline.yml will not be reflected.
-          GITHUB_TOKEN: ${{ steps.app-token-generation.outputs.token }}
-
-      - name: Comment on dependabot PR
-        if: github.event.action == 'opened' && !contains(github.event.pull_request.labels.*.name, 'CI Cleared') && github.event.pull_request.user.id == 49699333
-        uses: thollander/actions-comment-pull-request@fabd468d3a1a0b97feee5f6b9e499eab0dd903f6
-        with:
-          message: Set the milestone to the next ${{ (github.head_ref == 'master' && 'patch') || 'minor' }} version, check for supply chain attacks, and then add the `CI Cleared` label to allow CI to run.
-          GITHUB_TOKEN: ${{ steps.app-token-generation.outputs.token }}
-
       - name: "Remove Stale 'CI Cleared' Label"
         if: github.event.action == 'synchronize' || github.event.action == 'reopened'
         uses: actions-ecosystem/action-remove-labels@2ce5d41b4b6aa8503e285553f75ed56e0a40bae0

diff --git a/.github/workflows/new-pr-comments.yml b/.github/workflows/new-pr-comments.yml
@@ -0,0 +1,36 @@
+name: CI Gated PR Comments
+
+on:
+  pull_request_target:
+    types:
+      - opened
+    branches:
+      - dev
+      - master
+
+jobs:
+  comment-on-new-pr:
+    name: Comment New PR
+    if: github.event.pull_request.head.repo.id != github.event.pull_request.base.repo.id || github.event.pull_request.user.id == 49699333
+    runs-on: ubuntu-latest
+    steps:
+      - name: Generate App Token
+        id: app-token-generation
+        uses: actions/create-github-app-token@v1
+        with:
+          app-id: ${{ secrets.APP_ID }}
+          private-key: ${{ secrets.APP_PRIVATE_KEY }}
+
+      - name: Comment on new Fork PR
+        if: github.event.pull_request.user.id != 49699333
+        uses: thollander/actions-comment-pull-request@fabd468d3a1a0b97feee5f6b9e499eab0dd903f6
+        with:
+          message: Thank you for contributing to ${{ github.event.pull_request.base.repo.name }}! The workflow '${{ github.workflow }}' requires repository secrets and will not run without approval. Maintainers can add the `CI Cleared` label to allow it to run. Note that any changes to ci-security.yml and ci-pipeline.yml will not be reflected.
+          GITHUB_TOKEN: ${{ steps.app-token-generation.outputs.token }}
+
+      - name: Comment on dependabot PR
+        if: github.event.pull_request.user.id == 49699333
+        uses: thollander/actions-comment-pull-request@fabd468d3a1a0b97feee5f6b9e499eab0dd903f6
+        with:
+          message: Set the milestone to the next ${{ (github.head_ref == 'master' && 'patch') || 'minor' }} version, check for supply chain attacks, and then add the `CI Cleared` label to allow CI to run.
+          GITHUB_TOKEN: ${{ steps.app-token-generation.outputs.token }}
diff --git a/src/Tgstation.Server.Host/Core/Application.cs b/src/Tgstation.Server.Host/Core/Application.cs
@@ -9,6 +9,7 @@
 using Elastic.CommonSchema.Serilog;
 
 using HotChocolate.AspNetCore;
+using HotChocolate.Subscriptions;
 using HotChocolate.Types;
 
 using Microsoft.AspNetCore.Authentication;
@@ -296,7 +297,7 @@ void ConfigureNewtonsoftJsonSerializerSettingsForApi(JsonSerializerSettings sett
 			// configure graphql
 			if (postSetupServices.InternalConfiguration.EnableGraphQL)
 				services
-					.AddScoped<ITopicEventReceiver, ShutdownAwareTopicEventReceiver>()
+					.AddScoped<GraphQL.Subscriptions.ITopicEventReceiver, ShutdownAwareTopicEventReceiver>()
 					.AddGraphQLServer()
 					.AddAuthorization()
 					.ModifyOptions(options =>
@@ -311,7 +312,11 @@ void ConfigureNewtonsoftJsonSerializerSettingsForApi(JsonSerializerSettings sett
 					})
 #endif
 					.AddMutationConventions()
-					.AddInMemorySubscriptions()
+					.AddInMemorySubscriptions(
+						new SubscriptionOptions
+						{
+							TopicBufferCapacity = 1024, // mainly so high for tests, not possible to DoS the server without authentication and some other access to generate messages
+						})
 					.AddGlobalObjectIdentification()
 					.AddQueryFieldToMutationPayloads()
 					.ModifyOptions(options =>

diff --git a/tests/Tgstation.Server.Tests/Live/HoldLastObserver.cs b/tests/Tgstation.Server.Tests/Live/HoldLastObserver.cs
@@ -1,4 +1,5 @@
 using System;
+using System.Threading;
 
 namespace Tgstation.Server.Tests.Live
 {
@@ -10,9 +11,12 @@ sealed class HoldLastObserver<T> : IObserver<T>
 
 		public T LastValue { get; private set; }
 
-		public ulong ErrorCount { get; private set; }
+		public ulong ErrorCount => errorCount;
 
-		public ulong ResultCount { get; private set; }
+		public ulong ResultCount => resultCount;
+
+		ulong errorCount;
+		ulong resultCount;
 
 		public void OnCompleted()
 		{
@@ -21,13 +25,13 @@ public void OnCompleted()
 
 		public void OnError(Exception error)
 		{
-			++ErrorCount;
+			Interlocked.Increment(ref errorCount);
 			LastError = error;
 		}
 
 		public void OnNext(T value)
 		{
-			++ResultCount;
+			Interlocked.Increment(ref resultCount);
 			LastValue = value;
 		}
 	}

diff --git a/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs b/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs
@@ -45,6 +45,7 @@
 using Tgstation.Server.Host.Extensions;
 using Tgstation.Server.Host.Jobs;
 using Tgstation.Server.Host.System;
+using Tgstation.Server.Host.Utils;
 using Tgstation.Server.Tests.Live.Instance;
 
 namespace Tgstation.Server.Tests.Live
@@ -54,19 +55,17 @@ namespace Tgstation.Server.Tests.Live
 	[TestCategory("RequiresDatabase")]
 	public sealed class TestLiveServer
 	{
-		const ushort InitialPort = 42069;
 		public static readonly Version TestUpdateVersion = new(5, 11, 0);
 
 		static readonly Lazy<ushort> odDMPort = new(() => FreeTcpPort());
-		static readonly Lazy<ushort> odDDPort = new(() => FreeTcpPort());
-		static readonly Lazy<ushort> compatDMPort = new(() => FreeTcpPort());
-		static readonly Lazy<ushort> compatDDPort = new(() => FreeTcpPort());
-		static readonly Lazy<ushort> mainDDPort = new(() => FreeTcpPort());
-		static readonly Lazy<ushort> mainDMPort = new(() => FreeTcpPort());
+		static readonly Lazy<ushort> odDDPort = new(() => FreeTcpPort(odDMPort.Value));
+		static readonly Lazy<ushort> compatDMPort = new(() => FreeTcpPort(odDDPort.Value, odDMPort.Value));
+		static readonly Lazy<ushort> compatDDPort = new(() => FreeTcpPort(odDDPort.Value, odDMPort.Value, compatDMPort.Value));
+		static readonly Lazy<ushort> mainDDPort = new(() => FreeTcpPort(odDDPort.Value, odDMPort.Value, compatDMPort.Value, compatDDPort.Value));
+		static readonly Lazy<ushort> mainDMPort = new(() => FreeTcpPort(odDDPort.Value, odDMPort.Value, compatDMPort.Value, compatDDPort.Value, mainDDPort.Value));
 
 		static void InitializePorts()
 		{
-			tcpPortCounter = InitialPort;
 			_ = odDMPort.Value;
 			_ = odDDPort.Value;
 			_ = compatDMPort.Value;
@@ -166,14 +165,41 @@ static bool TerminateAllEngineServers()
 			return result;
 		}
 
-		static int tcpPortCounter = InitialPort;
-
-		static ushort FreeTcpPort()
+		static ushort FreeTcpPort(params ushort[] usedPorts)
 		{
-			var result = Interlocked.Increment(ref tcpPortCounter);
+			ushort result;
+			var listeners = new List<TcpListener>();
+
+			try
+			{
+				do
+				{
+					var l = new TcpListener(IPAddress.Any, 0);
+					l.Start();
+					try
+					{
+						listeners.Add(l);
+					}
+					catch
+					{
+						using (l)
+							l.Stop();
+						throw;
+					}
+
+					result = (ushort)((IPEndPoint)l.LocalEndpoint).Port;
+				}
+				while (usedPorts.Contains(result) || result < 20000);
+			}
+			finally
+			{
+				foreach (var l in listeners)
+					using (l)
+						l.Stop();
+			}
 
 			Console.WriteLine($"Allocated port: {result}");
-			return (ushort)result;
+			return result;
 		}
 
 		[ClassInitialize]

diff --git a/tests/Tgstation.Server.Tests/Live/UsersTest.cs b/tests/Tgstation.Server.Tests/Live/UsersTest.cs
@@ -50,7 +50,21 @@ await ValueTaskExtensions.WhenAll(
 
 			Assert.IsFalse(observer.Completed);
 			Assert.AreEqual(0U, observer.ErrorCount);
-			Assert.AreEqual(new PlatformIdentifier().IsWindows ? 108U : 107U, observer.ResultCount); // sys user
+
+			var expected = new PlatformIdentifier().IsWindows ? 108U : 107U;
+			// wait up to 10s
+			var lastSeen = observer.ResultCount;
+			for (var i = 0; i < 10 && observer.ResultCount < expected; ++i)
+			{
+				await Task.Delay(TimeSpan.FromSeconds(1), cancellationToken);
+				if (observer.ResultCount > lastSeen)
+				{
+					lastSeen = observer.ResultCount;
+					i = -1;
+				}
+			}
+
+			Assert.AreEqual(expected, observer.ResultCount); // sys user
 			observer.LastValue.EnsureNoErrors();
 		}