From 96a4f6e0d9c51e7ff76e054a2459cf64bc301b08 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 22 May 2020 00:26:41 -0400 Subject: [PATCH 1/8] Ensure revision information is present before posting test merge comments --- .../Components/Deployment/DmbFactory.cs | 2 +- .../Components/Deployment/DreamMaker.cs | 20 ++++++++++--------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Deployment/DmbFactory.cs b/src/Tgstation.Server.Host/Components/Deployment/DmbFactory.cs index 9c6b2190de..281bb892e9 100644 --- a/src/Tgstation.Server.Host/Components/Deployment/DmbFactory.cs +++ b/src/Tgstation.Server.Host/Components/Deployment/DmbFactory.cs @@ -196,6 +196,7 @@ namespace Tgstation.Server.Host.Components.Deployment throw new ArgumentNullException(nameof(compileJob)); // ensure we have the entire compile job tree + logger.LogTrace("Loading compile job {0}...", compileJob.Id); await databaseContextFactory.UseContext(async db => compileJob = await db.CompileJobs.Where(x => x.Id == compileJob.Id) .Include(x => x.Job).ThenInclude(x => x.StartedBy) .Include(x => x.RevisionInformation).ThenInclude(x => x.PrimaryTestMerge).ThenInclude(x => x.MergedBy) @@ -211,7 +212,6 @@ namespace Tgstation.Server.Host.Components.Deployment compileJob.Job.StoppedAt = DateTimeOffset.Now; } - logger.LogTrace("Loading compile job {0}...", compileJob.Id); var providerSubmitted = false; var newProvider = new DmbProvider(compileJob, ioManager, () => { diff --git a/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs b/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs index 9d19b127e7..2cceaaa288 100644 --- a/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs +++ b/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs @@ -623,7 +623,8 @@ namespace Tgstation.Server.Host.Components.Deployment Instance = new Models.Instance { Id = metadata.Id - } + }, + ActiveTestMerges = new List() }; logger.LogWarning(Repository.Repository.OriginTrackingErrorTemplate, repoSha); @@ -664,12 +665,9 @@ namespace Tgstation.Server.Host.Components.Deployment Id = revInfo.Id }; - databaseContext.RevisionInformations.Attach(compileJob.RevisionInformation); databaseContext.Jobs.Attach(compileJob.Job); databaseContext.CompileJobs.Add(compileJob); - await PostDeploymentComments(compileJob, repositorySettings, repoOwner, repoName).ConfigureAwait(false); - // The difficulty with compile jobs is they have a two part commit await databaseContext.Save(cancellationToken).ConfigureAwait(false); try @@ -692,6 +690,10 @@ namespace Tgstation.Server.Host.Components.Deployment throw; } + // set the compile job revinfo back to the full tree, so that it may be used by this call + compileJob.RevisionInformation = revInfo; + await PostDeploymentComments(compileJob, repositorySettings, repoOwner, repoName).ConfigureAwait(false); + await eventConsumer.HandleEvent(EventType.DeploymentComplete, null, cancellationToken).ConfigureAwait(false); await chatManager.SendUpdateMessage( @@ -849,13 +851,13 @@ namespace Tgstation.Server.Host.Components.Deployment // removed prs foreach (var I in outgoingCompileJob + .RevisionInformation + .ActiveTestMerges + .Select(x => x.TestMerge) + .Where(x => !compileJob .RevisionInformation .ActiveTestMerges - .Select(x => x.TestMerge) - .Where(x => !compileJob - .RevisionInformation - .ActiveTestMerges - .Any(y => y.TestMerge.Number == x.Number))) + .Any(y => y.TestMerge.Number == x.Number))) tasks.Add(CommentOnPR(I.Number, "#### Test Merge Removed")); // updated prs From 86f887f71364b86843672e72ae48c4df8d126fd7 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 22 May 2020 00:39:44 -0400 Subject: [PATCH 2/8] Version bump --- build/Version.props | 2 +- src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/build/Version.props b/build/Version.props index 8ced6bea56..b09d27300e 100644 --- a/build/Version.props +++ b/build/Version.props @@ -2,7 +2,7 @@ - 4.2.4 + 4.2.5 6.4.0 6.3.0 5.2.1 diff --git a/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs b/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs index 2cceaaa288..a4d858cf1c 100644 --- a/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs +++ b/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs @@ -666,6 +666,7 @@ namespace Tgstation.Server.Host.Components.Deployment }; databaseContext.Jobs.Attach(compileJob.Job); + databaseContext.RevisionInformations.Attach(compileJob.RevisionInformation); databaseContext.CompileJobs.Add(compileJob); // The difficulty with compile jobs is they have a two part commit From 77c8be2086597a0887a63c4d3cf22d6a047de599 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 22 May 2020 11:02:25 -0400 Subject: [PATCH 3/8] Fix GitHub comments more --- src/Tgstation.Server.Api/Models/ErrorCode.cs | 6 ++ .../Components/Deployment/DreamMaker.cs | 67 ++++++++++--------- 2 files changed, 41 insertions(+), 32 deletions(-) diff --git a/src/Tgstation.Server.Api/Models/ErrorCode.cs b/src/Tgstation.Server.Api/Models/ErrorCode.cs index 7b34459278..1f8141301d 100644 --- a/src/Tgstation.Server.Api/Models/ErrorCode.cs +++ b/src/Tgstation.Server.Api/Models/ErrorCode.cs @@ -483,5 +483,11 @@ namespace Tgstation.Server.Api.Models /// [Description("Could not bind to requested DreamDaemon port! Is there another service running on that port?")] DreamDaemonPortInUse, + + /// + /// Failed to post GitHub comments, send chat message, or send TGS event. + /// + [Description("The deployment succeeded but one or more notification events failed!")] + PostDeployFailure, } } \ No newline at end of file diff --git a/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs b/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs index a4d858cf1c..a10d0cb2ef 100644 --- a/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs +++ b/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs @@ -651,6 +651,7 @@ namespace Tgstation.Server.Host.Components.Deployment cancellationToken) .ConfigureAwait(false); + var activeCompileJob = compileJobConsumer.LatestCompileJob(); try { await databaseContextFactory.UseContext( @@ -691,19 +692,30 @@ namespace Tgstation.Server.Host.Components.Deployment throw; } - // set the compile job revinfo back to the full tree, so that it may be used by this call - compileJob.RevisionInformation = revInfo; - await PostDeploymentComments(compileJob, repositorySettings, repoOwner, repoName).ConfigureAwait(false); + var commentsTask = PostDeploymentComments( + revInfo, + activeCompileJob?.RevisionInformation, + repositorySettings, + repoOwner, + repoName); - await eventConsumer.HandleEvent(EventType.DeploymentComplete, null, cancellationToken).ConfigureAwait(false); + var eventTask = eventConsumer.HandleEvent(EventType.DeploymentComplete, null, cancellationToken); - await chatManager.SendUpdateMessage( + var chatTask = chatManager.SendUpdateMessage( String.Format( CultureInfo.InvariantCulture, "Deployment complete! Changes will be applied when DreamDaemon {0}.", watchdog.Running ? "reboots" : "is launched"), - cancellationToken) - .ConfigureAwait(false); + cancellationToken); + + try + { + await Task.WhenAll(commentsTask, eventTask, chatTask).ConfigureAwait(false); + } + catch (Exception ex) + { + throw new JobException(ErrorCode.PostDeployFailure, ex); + } } #pragma warning restore CA1506 @@ -785,13 +797,15 @@ namespace Tgstation.Server.Host.Components.Deployment /// /// Post deployment GitHub comments. /// - /// The deployed . + /// The deployed . + /// The of the previous deployment. /// The . /// The GitHub repostiory owner. /// The GitHub repostiory name. /// A representing the running operation. async Task PostDeploymentComments( - Models.CompileJob compileJob, + Models.RevisionInformation deployedRevisionInformation, + Models.RevisionInformation previousRevisionInformation, Models.RepositorySettings repositorySettings, string repoOwner, string repoName) @@ -799,18 +813,13 @@ namespace Tgstation.Server.Host.Components.Deployment if (repositorySettings?.AccessToken == null) return; - // potential for commenting on a test merge change - var outgoingCompileJob = compileJobConsumer.LatestCompileJob(); - - if ((outgoingCompileJob != null && outgoingCompileJob.RevisionInformation.CommitSha == compileJob.RevisionInformation.CommitSha) || !repositorySettings.PostTestMergeComment.Value) + if ((previousRevisionInformation != null && previousRevisionInformation.CommitSha == previousRevisionInformation.CommitSha) + || !repositorySettings.PostTestMergeComment.Value) return; - outgoingCompileJob ??= new Models.CompileJob + previousRevisionInformation = new Models.RevisionInformation { - RevisionInformation = new Models.RevisionInformation - { - ActiveTestMerges = new List() - } + ActiveTestMerges = new List() }; var gitHubClient = gitHubClientFactory.CreateClient(repositorySettings.AccessToken); @@ -836,38 +845,32 @@ namespace Tgstation.Server.Host.Components.Deployment testMerge.Comment != null ? String.Format(CultureInfo.InvariantCulture, "{0}{0}##### Comment{0}{1}", Environment.NewLine, testMerge.Comment) : String.Empty, updated ? "Updated" : "Deployed", metadata.Name, - compileJob.RevisionInformation.OriginCommitSha, - compileJob.RevisionInformation.CommitSha); + deployedRevisionInformation.OriginCommitSha, + deployedRevisionInformation.CommitSha); // added prs - foreach (var I in compileJob - .RevisionInformation + foreach (var I in deployedRevisionInformation .ActiveTestMerges .Select(x => x.TestMerge) - .Where(x => !outgoingCompileJob - .RevisionInformation + .Where(x => !previousRevisionInformation .ActiveTestMerges .Any(y => y.TestMerge.Number == x.Number))) tasks.Add(CommentOnPR(I.Number, FormatTestMerge(I, false))); // removed prs - foreach (var I in outgoingCompileJob - .RevisionInformation + foreach (var I in previousRevisionInformation .ActiveTestMerges .Select(x => x.TestMerge) - .Where(x => !compileJob - .RevisionInformation + .Where(x => !deployedRevisionInformation .ActiveTestMerges .Any(y => y.TestMerge.Number == x.Number))) tasks.Add(CommentOnPR(I.Number, "#### Test Merge Removed")); // updated prs - foreach (var I in compileJob - .RevisionInformation + foreach (var I in deployedRevisionInformation .ActiveTestMerges .Select(x => x.TestMerge) - .Where(x => outgoingCompileJob - .RevisionInformation + .Where(x => previousRevisionInformation .ActiveTestMerges .Any(y => y.TestMerge.Number == x.Number))) tasks.Add(CommentOnPR(I.Number, FormatTestMerge(I, true))); From 54d6442dc7ff57fa05665e2d89afdc4f4b460f53 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 22 May 2020 11:08:50 -0400 Subject: [PATCH 4/8] Improve the issue template --- .github/ISSUE_TEMPLATE.md | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/.github/ISSUE_TEMPLATE.md b/.github/ISSUE_TEMPLATE.md index 861391feef..51139c105b 100644 --- a/.github/ISSUE_TEMPLATE.md +++ b/.github/ISSUE_TEMPLATE.md @@ -2,10 +2,16 @@ Note that this repository does not contain any client code (With the exception o Please include: -- A description of the issue. - - The version of tgstation-server you were using. -- Reproduction steps for the issue if possible (Very helpful). +- A description of the issue. -- Relevent server logs, request logs, and screenshots if possible. +- A link to your codebase git (if public) + - Include active SHA/test merges if applicable + +- The client you we're using (Desktop control panel, web control panel, etc) + - Include a version if applicable + +- Reproduction steps for the issue if possible from your client. + +- The server log of when the event happened (The full file is much more useful than snippets). From 2174ab2464b90c275219f3ac14b41f8a3ecb2d8f Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 22 May 2020 11:23:29 -0400 Subject: [PATCH 5/8] Limit log files to 50MB --- src/Tgstation.Server.Host/Core/Application.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Tgstation.Server.Host/Core/Application.cs b/src/Tgstation.Server.Host/Core/Application.cs index bc51e04c77..51bb634cb5 100644 --- a/src/Tgstation.Server.Host/Core/Application.cs +++ b/src/Tgstation.Server.Host/Core/Application.cs @@ -126,9 +126,9 @@ namespace Tgstation.Server.Host.Core var logPath = !String.IsNullOrEmpty(postSetupServices.FileLoggingConfiguration.Directory) ? postSetupServices.FileLoggingConfiguration.Directory : IOManager.ConcatPath( - Environment.GetFolderPath(Environment.SpecialFolder.CommonApplicationData), - AssemblyInformationProvider.VersionPrefix, - "Logs"); + Environment.GetFolderPath(Environment.SpecialFolder.CommonApplicationData), + AssemblyInformationProvider.VersionPrefix, + "Logs"); var logEventLevel = ConvertSeriLogLevel(postSetupServices.FileLoggingConfiguration.LogLevel); @@ -141,6 +141,7 @@ namespace Tgstation.Server.Host.Core formatter, logPath, logEventLevel ?? LogEventLevel.Verbose, + 50 * 1024 * 1024, // 50MB max size flushToDiskInterval: TimeSpan.FromSeconds(2)); }); From f174dfa63a6e7c6483e51174bcbae06f8b9da615 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 22 May 2020 11:26:11 -0400 Subject: [PATCH 6/8] Cap integration tests to 10 minutes so we can better diagnose why Travis hangs --- tests/Tgstation.Server.Tests/IntegrationTest.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Tgstation.Server.Tests/IntegrationTest.cs b/tests/Tgstation.Server.Tests/IntegrationTest.cs index 06ea881442..90de88be80 100644 --- a/tests/Tgstation.Server.Tests/IntegrationTest.cs +++ b/tests/Tgstation.Server.Tests/IntegrationTest.cs @@ -115,6 +115,7 @@ namespace Tgstation.Server.Tests using var server = new TestingServer(); using var serverCts = new CancellationTokenSource(); + serverCts.CancelAfter(TimeSpan.FromMinutes(10)); var cancellationToken = serverCts.Token; TerminateAllDDs(); var serverTask = server.Run(cancellationToken); From a401f1afa9f0357dfe2ad0217a35d094c1f2fa97 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 22 May 2020 11:30:18 -0400 Subject: [PATCH 7/8] Pull request template update --- .github/PULL_REQUEST_TEMPLATE.md | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 98fe22ab33..42203e2250 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1,13 +1,19 @@ -[Release Notes]: # (Your PR should contain a detailed list of notable changes, titled appropriately. This includes any observable changes to the server or DMAPI. See example below) +[Release Notes]: # (Your PR should contain a detailed list of notable changes, titled appropriately. This includes any observable changes to the server or DMAPI. See examples below) :cl: -Description of your change -Each newline corresponds to a release note in the upcoming sprint +Description of your change. +Each newline corresponds to a release note in the release your change is included in. /:cl: :cl: -You can also have multiple sets of release notes per pull request -They will be amalgamated together in the end +You can also have multiple sets of release notes per pull request. +They will be amalgamated together in the end. /:cl: -[Why]: # (Please add a short description [two lines down] of why you think these changes would benefit the game. If you can't justify it in words, it might not be worth adding.) +:cl: Categories +Categories are divided up in the release notes and set after the :cl: header. +The default category is Core. +The current standard categories are Core, DMAPI, HTTP API, and Host Watchdog. +/:cl: + +[Why]: # (If this does not close or work on an existing GitHub issue, please add a short description [two lines down] of why you think these changes would benefit the server. If you can't justify it in words, it might not be worth adding.) From f4669570072aad70b8d72c17dcf45f76066f985f Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 22 May 2020 11:38:37 -0400 Subject: [PATCH 8/8] Slightly shorter timespan as we need to not hit the Travis timeout --- src/Tgstation.Server.Host/Components/README.md | 6 ++++-- tests/Tgstation.Server.Tests/IntegrationTest.cs | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/README.md b/src/Tgstation.Server.Host/Components/README.md index 834e183de8..6d48a22aae 100644 --- a/src/Tgstation.Server.Host/Components/README.md +++ b/src/Tgstation.Server.Host/Components/README.md @@ -8,13 +8,15 @@ Component code is where the magic and tears of TGS are made. There are six main - [The Watchdog](./Watchdog) - [The configuration system](./StaticFiles) -There exist two more namespaces in here that don't fit in these 6. +There exist two more namespaces in here that don't directly fit in these 6 components. - [Interop](./Interop) deals with the bulk of DMAPI communication (Though it's not all contained here). -- [Session](./Session) contains the classes used for actually executing DreamDaemon among other things. +- [Session](./Session) contains the classes used for actually executing DreamDaemon, sending topic requests, receiving bridge requests, among other things. Each of these is tied under the roof of an [IInstance](./IInstance.cs) ([implementation](./Instance.cs)). While the database represents stored instance data, in component code an instance is online, or doesn't exist. `IInstance`s are created via the [IInstanceFactory](./IInstanceFactory.cs) ([implementation](./InstanceFactory.cs)) and are generally controlled via the [IInstanceManager](./IInstanceManager.cs) ([implementation](./InstanceManager.cs)). + +Many classes in here implement [IHostedService](), `InstanceManager` being the only one that is called by the ASP.NET runtime. In the case of instances `StartAsync()` is called when an `Instance` is being brought online (from server startup or user request). The `Instance` handles calling `StartAsync()` on its various subcomponents that need it. When an `Instance` is being brought offline (from server shutdown/restart/update or user request) the same pattern is followed calling `StopAsync()`. diff --git a/tests/Tgstation.Server.Tests/IntegrationTest.cs b/tests/Tgstation.Server.Tests/IntegrationTest.cs index 90de88be80..26fcdd91d8 100644 --- a/tests/Tgstation.Server.Tests/IntegrationTest.cs +++ b/tests/Tgstation.Server.Tests/IntegrationTest.cs @@ -115,7 +115,7 @@ namespace Tgstation.Server.Tests using var server = new TestingServer(); using var serverCts = new CancellationTokenSource(); - serverCts.CancelAfter(TimeSpan.FromMinutes(10)); + serverCts.CancelAfter(new TimeSpan(0, 9, 30)); var cancellationToken = serverCts.Token; TerminateAllDDs(); var serverTask = server.Run(cancellationToken);