Fix auto updates not clearing merged PRs

This commit is contained in:
Jordan Brown
2020-07-27 22:23:58 -04:00
parent 6399c4cbf1
commit 722567ded8
6 changed files with 205 additions and 17 deletions
+103 -11
View File
@@ -1,5 +1,6 @@
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Logging;
using Octokit;
using Serilog.Context;
using System;
using System.Collections.Generic;
@@ -13,6 +14,9 @@ using Tgstation.Server.Host.Components.Deployment;
using Tgstation.Server.Host.Components.Events;
using Tgstation.Server.Host.Components.Repository;
using Tgstation.Server.Host.Components.Watchdog;
using Tgstation.Server.Host.Configuration;
using Tgstation.Server.Host.Core;
using Tgstation.Server.Host.Extensions;
using Tgstation.Server.Host.Jobs;
using Tgstation.Server.Host.Models;
@@ -60,6 +64,11 @@ namespace Tgstation.Server.Host.Components
/// </summary>
readonly IEventConsumer eventConsumer;
/// <summary>
/// The <see cref="IGitHubClientFactory"/> for the <see cref="Instance"/>.
/// </summary>
readonly IGitHubClientFactory gitHubClientFactory;
/// <summary>
/// The <see cref="ILogger"/> for the <see cref="Instance"/>
/// </summary>
@@ -70,6 +79,11 @@ namespace Tgstation.Server.Host.Components
/// </summary>
readonly Api.Models.Instance metadata;
/// <summary>
/// The <see cref="GeneralConfiguration"/> for the <see cref="Instance"/>.
/// </summary>
readonly GeneralConfiguration generalConfiguration;
/// <summary>
/// <see langword="lock"/> <see cref="object"/> for <see cref="timerCts"/> and <see cref="timerTask"/>.
/// </summary>
@@ -98,7 +112,9 @@ namespace Tgstation.Server.Host.Components
/// <param name="dmbFactory">The value of <see cref="dmbFactory"/></param>
/// <param name="jobManager">The value of <see cref="jobManager"/></param>
/// <param name="eventConsumer">The value of <see cref="eventConsumer"/></param>
/// <param name="gitHubClientFactory">The value of <see cref="gitHubClientFactory"/>.</param>
/// <param name="logger">The value of <see cref="logger"/></param>
/// <param name="generalConfiguration">The value of <see cref="generalConfiguration"/>.</param>
public Instance(
Api.Models.Instance metadata,
IRepositoryManager repositoryManager,
@@ -111,7 +127,9 @@ namespace Tgstation.Server.Host.Components
IDmbFactory dmbFactory,
IJobManager jobManager,
IEventConsumer eventConsumer,
ILogger<Instance> logger)
IGitHubClientFactory gitHubClientFactory,
ILogger<Instance> logger,
GeneralConfiguration generalConfiguration)
{
this.metadata = metadata ?? throw new ArgumentNullException(nameof(metadata));
RepositoryManager = repositoryManager ?? throw new ArgumentNullException(nameof(repositoryManager));
@@ -123,7 +141,9 @@ namespace Tgstation.Server.Host.Components
this.dmbFactory = dmbFactory ?? throw new ArgumentNullException(nameof(dmbFactory));
this.jobManager = jobManager ?? throw new ArgumentNullException(nameof(jobManager));
this.eventConsumer = eventConsumer ?? throw new ArgumentNullException(nameof(eventConsumer));
this.gitHubClientFactory = gitHubClientFactory ?? throw new ArgumentNullException(nameof(gitHubClientFactory));
this.logger = logger ?? throw new ArgumentNullException(nameof(logger));
this.generalConfiguration = generalConfiguration ?? throw new ArgumentNullException(nameof(generalConfiguration));
timerLock = new object();
}
@@ -230,7 +250,7 @@ namespace Tgstation.Server.Host.Components
.Include(x => x.ActiveTestMerges).ThenInclude(x => x.TestMerge)
.FirstOrDefaultAsync(jobCancellationToken);
async Task UpdateRevInfo(string currentHead, bool onOrigin)
async Task UpdateRevInfo(string currentHead, bool onOrigin, IEnumerable<RevInfoTestMerge> updatedTestMerges)
{
if (currentRevInfo == null)
currentRevInfo = await LoadRevInfo().ConfigureAwait(false);
@@ -253,7 +273,8 @@ namespace Tgstation.Server.Host.Components
Instance = attachedInstance
};
if (!onOrigin)
currentRevInfo.ActiveTestMerges = new List<RevInfoTestMerge>(oldRevInfo.ActiveTestMerges);
currentRevInfo.ActiveTestMerges = new List<RevInfoTestMerge>(
updatedTestMerges ?? oldRevInfo.ActiveTestMerges);
databaseContext.Instances.Attach(attachedInstance);
databaseContext.RevisionInformations.Add(currentRevInfo);
@@ -261,8 +282,9 @@ namespace Tgstation.Server.Host.Components
}
// take appropriate auto update actions
bool shouldSyncTracked;
if (repositorySettings.AutoUpdatesKeepTestMerges.Value)
bool shouldSyncTracked = false;
bool preserveTestMerges = repositorySettings.AutoUpdatesKeepTestMerges.Value;
if (preserveTestMerges)
{
logger.LogTrace("Preserving test merges...");
@@ -275,21 +297,29 @@ namespace Tgstation.Server.Host.Components
currentRevInfo = await currentRevInfoTask.ConfigureAwait(false);
var updatedTestMerges = await RemoveMergedPullRequests(
repo,
repositorySettings,
currentRevInfo,
cancellationToken)
.ConfigureAwait(false);
var lastRevInfoWasOriginCommit = currentRevInfo == default || currentRevInfo.CommitSha == currentRevInfo.OriginCommitSha;
var stillOnOrigin = result.Value && lastRevInfoWasOriginCommit;
var currentHead = repo.Head;
if (currentHead != startSha)
{
await UpdateRevInfo(currentHead, stillOnOrigin).ConfigureAwait(false);
await UpdateRevInfo(currentHead, stillOnOrigin, updatedTestMerges).ConfigureAwait(false);
shouldSyncTracked = stillOnOrigin;
}
else
shouldSyncTracked = false;
}
else
if (!preserveTestMerges)
{
logger.LogTrace("Not preserving test merges...");
logger.LogTrace("Resetting to origin...");
await repo.ResetToOrigin(NextProgressReporter(), jobCancellationToken).ConfigureAwait(false);
var currentHead = repo.Head;
@@ -301,7 +331,7 @@ namespace Tgstation.Server.Host.Components
.ConfigureAwait(false);
if (currentHead != startSha && currentRevInfo == default)
await UpdateRevInfo(currentHead, true).ConfigureAwait(false);
await UpdateRevInfo(currentHead, true, null).ConfigureAwait(false);
shouldSyncTracked = true;
}
@@ -312,7 +342,7 @@ namespace Tgstation.Server.Host.Components
var pushedOrigin = await repo.Sychronize(repositorySettings.AccessUser, repositorySettings.AccessToken, repositorySettings.CommitterName, repositorySettings.CommitterEmail, NextProgressReporter(), shouldSyncTracked, jobCancellationToken).ConfigureAwait(false);
var currentHead = repo.Head;
if (currentHead != currentRevInfo.CommitSha)
await UpdateRevInfo(currentHead, pushedOrigin).ConfigureAwait(false);
await UpdateRevInfo(currentHead, pushedOrigin, null).ConfigureAwait(false);
}
repoHead = repo.Head;
@@ -393,7 +423,69 @@ namespace Tgstation.Server.Host.Components
logger.LogTrace("Leaving auto update loop...");
}
#pragma warning restore CA1502
#pragma warning restore CA1502
/// <summary>
/// Get the updated list of <see cref="TestMerge"/>s for an origin merge.
/// </summary>
/// <param name="repository">The <see cref="IRepository"/> to use.</param>
/// <param name="repositorySettings">The <see cref="RepositorySettings"/>.</param>
/// <param name="revisionInformation">The current <see cref="RevisionInformation"/>.</param>
/// <param name="cancellationToken">The <see cref="CancellationToken"/> for the operation.</param>
/// <returns>A <see cref="Task{TResult}"/> resulting in the <see cref="IReadOnlyCollection{T}"/> of <see cref="RevInfoTestMerge"/>s that should remain the new <see cref="RevisionInformation"/>.</returns>
async Task<IReadOnlyCollection<RevInfoTestMerge>> RemoveMergedPullRequests(
IRepository repository,
RepositorySettings repositorySettings,
RevisionInformation revisionInformation,
CancellationToken cancellationToken)
{
if (revisionInformation.ActiveTestMerges?.Any() != true)
{
logger.LogTrace("No test merges to remove.");
return Array.Empty<RevInfoTestMerge>();
}
var gitHubClient = repositorySettings.AccessToken != null
? gitHubClientFactory.CreateClient(repositorySettings.AccessToken)
: (String.IsNullOrEmpty(generalConfiguration.GitHubAccessToken)
? gitHubClientFactory.CreateClient()
: gitHubClientFactory.CreateClient(generalConfiguration.GitHubAccessToken));
var tasks = revisionInformation
.ActiveTestMerges
.Select(x => gitHubClient
.PullRequest
.Get(repository.GitHubOwner, repository.GitHubRepoName, x.TestMerge.Number)
.WithToken(cancellationToken));
try
{
await Task.WhenAll(tasks).ConfigureAwait(false);
}
catch (Exception ex) when (!(ex is OperationCanceledException))
{
logger.LogWarning(ex, "Pull requests update check failed!");
}
var newList = revisionInformation.ActiveTestMerges.ToList();
async Task CheckRemovePR(Task<PullRequest> task)
{
var pr = await task.ConfigureAwait(false);
if (!pr.Merged)
return;
// We don't just assume, actually check the repo contains the merge commit.
if (await repository.ShaIsParent(pr.MergeCommitSha, cancellationToken).ConfigureAwait(false))
newList.Remove(
newList.First(
potential => potential.TestMerge.Number == pr.Number));
}
foreach (var prTask in tasks)
await CheckRemovePR(prTask).ConfigureAwait(false);
return newList;
}
/// <inheritdoc />
public Task InstanceRenamed(string newName, CancellationToken cancellationToken)
@@ -1,4 +1,5 @@
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using System;
using System.Threading;
using System.Threading.Tasks;
@@ -11,6 +12,7 @@ using Tgstation.Server.Host.Components.Interop.Bridge;
using Tgstation.Server.Host.Components.Repository;
using Tgstation.Server.Host.Components.Session;
using Tgstation.Server.Host.Components.Watchdog;
using Tgstation.Server.Host.Configuration;
using Tgstation.Server.Host.Core;
using Tgstation.Server.Host.Database;
using Tgstation.Server.Host.IO;
@@ -123,6 +125,11 @@ namespace Tgstation.Server.Host.Components
/// </summary>
readonly IServerPortProvider serverPortProvider;
/// <summary>
/// The <see cref="GeneralConfiguration"/> for the <see cref="InstanceFactory"/>.
/// </summary>
readonly GeneralConfiguration generalConfiguration;
/// <summary>
/// Construct an <see cref="InstanceFactory"/>
/// </summary>
@@ -146,6 +153,7 @@ namespace Tgstation.Server.Host.Components
/// <param name="repositoryFactory">The value of <see cref="repositoryFactory"/>.</param>
/// <param name="repositoryCommands">The value of <see cref="repositoryCommands"/>.</param>
/// <param name="serverPortProvider">The value of <see cref="serverPortProvider"/>.</param>
/// <param name="generalConfigurationOptions">The <see cref="IOptions{TOptions}"/> containing the value of <see cref="generalConfiguration"/>.</param>
public InstanceFactory(
IIOManager ioManager,
IDatabaseContextFactory databaseContextFactory,
@@ -166,7 +174,8 @@ namespace Tgstation.Server.Host.Components
IPlatformIdentifier platformIdentifier,
ILibGit2RepositoryFactory repositoryFactory,
ILibGit2Commands repositoryCommands,
IServerPortProvider serverPortProvider)
IServerPortProvider serverPortProvider,
IOptions<GeneralConfiguration> generalConfigurationOptions)
{
this.ioManager = ioManager ?? throw new ArgumentNullException(nameof(ioManager));
this.databaseContextFactory = databaseContextFactory ?? throw new ArgumentNullException(nameof(databaseContextFactory));
@@ -188,6 +197,7 @@ namespace Tgstation.Server.Host.Components
this.repositoryFactory = repositoryFactory ?? throw new ArgumentNullException(nameof(repositoryFactory));
this.repositoryCommands = repositoryCommands ?? throw new ArgumentNullException(nameof(repositoryCommands));
this.serverPortProvider = serverPortProvider ?? throw new ArgumentNullException(nameof(serverPortProvider));
generalConfiguration = generalConfigurationOptions?.Value ?? throw new ArgumentNullException(nameof(generalConfigurationOptions));
}
/// <inheritdoc />
@@ -287,7 +297,9 @@ namespace Tgstation.Server.Host.Components
dmbFactory,
jobManager,
eventConsumer,
loggerFactory.CreateLogger<Instance>());
gitHubClientFactory,
loggerFactory.CreateLogger<Instance>(),
generalConfiguration);
return instance;
}
@@ -1,4 +1,4 @@
using System;
using System;
using System.Threading;
using System.Threading.Tasks;
using Tgstation.Server.Api.Models;
@@ -132,5 +132,14 @@ namespace Tgstation.Server.Host.Components.Repository
/// <param name="cancellationToken">The <see cref="CancellationToken"/> for the operation</param>
/// <returns>A <see cref="Task"/> representing the running operation</returns>
Task CopyTo(string path, CancellationToken cancellationToken);
/// <summary>
/// Check if a given <paramref name="sha"/> is a parent of the current <see cref="Head"/>.
/// </summary>
/// <param name="sha">The SHA to check.</param>
/// <param name="cancellationToken">The <see cref="CancellationToken"/> for the operation.</param>
/// <returns>A <see cref="Task{TResult}"/> resulting in <see langword="true"/> if <paramref name="sha"/> is a parent of <see cref="Head"/>, <see langword="false"/> otherwise.</returns>
/// <remarks>This function is NOT reentrant.</remarks>
Task<bool> ShaIsParent(string sha, CancellationToken cancellationToken);
}
}
@@ -22,6 +22,16 @@ namespace Tgstation.Server.Host.Components.Repository
/// </summary>
public const string GitHubUrl = "://github.com/";
/// <summary>
/// The default username for committers.
/// </summary>
public const string DefaultCommitterName = "tgstation-server";
/// <summary>
/// The default password for committers.
/// </summary>
public const string DefaultCommitterEmail = "tgstation-server@users.noreply.github.com";
/// <summary>
/// Template error message for when tracking of the most recent origin commit fails
/// </summary>
@@ -554,7 +564,7 @@ namespace Tgstation.Server.Host.Components.Repository
trackedBranch = libGitRepo.Head.TrackedBranch;
logger.LogDebug("Merge origin/{2}: <{0} ({1})>", committerName, committerEmail, trackedBranch.FriendlyName);
result = libGitRepo.Merge(trackedBranch, new Signature(new Identity(committerName, committerEmail), DateTimeOffset.Now), new MergeOptions
result = libGitRepo.Merge(trackedBranch, new Signature(committerName, committerEmail, DateTimeOffset.Now), new MergeOptions
{
CommitOnSuccess = true,
FailOnConflict = true,
@@ -708,5 +718,43 @@ namespace Tgstation.Server.Host.Components.Repository
return true;
return false;
}, cancellationToken, TaskCreationOptions.LongRunning, TaskScheduler.Current);
/// <inheritdoc />
public Task<bool> ShaIsParent(string sha, CancellationToken cancellationToken) => Task.Factory.StartNew(() =>
{
var targetCommit = libGitRepo.Lookup<Commit>(sha);
if(targetCommit == null)
{
logger.LogTrace("Commit {0} not found in repository", sha);
return false;
}
cancellationToken.ThrowIfCancellationRequested();
var startSha = Head;
var mergeResult = libGitRepo.Merge(
targetCommit,
new Signature(
DefaultCommitterName,
DefaultCommitterEmail,
DateTimeOffset.Now),
new MergeOptions
{
FastForwardStrategy = FastForwardStrategy.FastForwardOnly,
FailOnConflict = true
});
if (mergeResult.Status == MergeStatus.UpToDate)
return true;
commands.Checkout(
libGitRepo,
new CheckoutOptions
{
CheckoutModifiers = CheckoutModifiers.Force
},
startSha);
return false;
}, cancellationToken, TaskCreationOptions.LongRunning, TaskScheduler.Current);
}
}
@@ -126,8 +126,8 @@ namespace Tgstation.Server.Host.Controllers
ChatBotLimit = initialSettings.ChatBotLimit ?? Models.Instance.DefaultChatBotLimit,
RepositorySettings = new RepositorySettings
{
CommitterEmail = "tgstation-server@users.noreply.github.com",
CommitterName = "tgstation-server",
CommitterEmail = Components.Repository.Repository.DefaultCommitterEmail,
CommitterName = Components.Repository.Repository.DefaultCommitterName,
PushTestMergeCommits = false,
ShowTestMergeCommitters = false,
AutoUpdatesKeepTestMerges = false,
@@ -19,6 +19,8 @@ using System.Threading.Tasks;
using Tgstation.Server.Api;
using Tgstation.Server.Api.Models;
using Tgstation.Server.Client;
using Tgstation.Server.Host.Components.Events;
using Tgstation.Server.Host.Components.Repository;
using Tgstation.Server.Host.Configuration;
using Tgstation.Server.Host.Database;
using Tgstation.Server.Host.Database.Migrations;
@@ -437,5 +439,30 @@ namespace Tgstation.Server.Tests
Assert.AreEqual(String.Empty, (await process.GetErrorOutput(default)).Trim());
Assert.AreEqual("Hello World!", (await process.GetStandardOutput(default)).Trim());
}
[TestMethod]
public async Task TestRepoParentLookup()
{
using var testingServer = new TestingServer();
LibGit2Sharp.Repository.Clone("https://github.com/Cyberboss/test", testingServer.Directory);
var libGit2Repo = new LibGit2Sharp.Repository(testingServer.Directory);
using var repo = new Host.Components.Repository.Repository(
libGit2Repo,
new LibGit2Commands(),
Mock.Of<Host.IO.IIOManager>(),
Mock.Of<IEventConsumer>(),
Mock.Of<ICredentialsProvider>(),
Mock.Of<ILogger<Host.Components.Repository.Repository>>(),
() => { });
const string StartSha = "af4da8beb9f9b374b04a3cc4d65acca662e8cc1a";
await repo.CheckoutObject(StartSha, progress => { }, default);
var result = await repo.ShaIsParent("2f8588a3ca0f6b027704a2a04381215619de3412", default);
Assert.IsTrue(result);
Assert.AreEqual(StartSha, repo.Head);
result = await repo.ShaIsParent("f636418bf47d238d33b0e4a34f0072b23a8aad0e", default);
Assert.IsFalse(result); ;
Assert.AreEqual(StartSha, repo.Head);
}
}
}