diff --git a/README.md b/README.md index ea51ba7f33..a646eda7c0 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ # tgstation-server -![CI Pipeline](https://github.com/tgstation/tgstation-server/workflows/CI%20Pipeline/badge.svg) [![codecov](https://codecov.io/gh/tgstation/tgstation-server/branch/master/graph/badge.svg)](https://codecov.io/gh/tgstation/tgstation-server) +[![CI Pipeline](https://github.com/tgstation/tgstation-server/actions/workflows/ci-pipeline.yml/badge.svg)](https://github.com/tgstation/tgstation-server/actions/workflows/ci-pipeline.yml) [![codecov](https://codecov.io/gh/tgstation/tgstation-server/branch/master/graph/badge.svg)](https://codecov.io/gh/tgstation/tgstation-server) [![GitHub license](https://img.shields.io/github/license/tgstation/tgstation-server.svg)](LICENSE) [![Average time to resolve an issue](http://isitmaintained.com/badge/resolution/tgstation/tgstation-server.svg)](http://isitmaintained.com/project/tgstation/tgstation-server "Average time to resolve an issue") [![NuGet version](https://img.shields.io/nuget/v/Tgstation.Server.Api.svg)](https://www.nuget.org/packages/Tgstation.Server.Api) [![NuGet version](https://img.shields.io/nuget/v/Tgstation.Server.Client.svg)](https://www.nuget.org/packages/Tgstation.Server.Client) diff --git a/src/Tgstation.Server.Host/Components/Engine/ByondInstallerBase.cs b/src/Tgstation.Server.Host/Components/Engine/ByondInstallerBase.cs index c7c73e3a23..40fbde1a40 100644 --- a/src/Tgstation.Server.Host/Components/Engine/ByondInstallerBase.cs +++ b/src/Tgstation.Server.Host/Components/Engine/ByondInstallerBase.cs @@ -146,7 +146,7 @@ namespace Tgstation.Server.Host.Components.Engine } /// - public override async ValueTask DownloadVersion(EngineVersion version, JobProgressReporter? progressReporter, CancellationToken cancellationToken) + public override async ValueTask DownloadVersion(EngineVersion version, JobProgressReporter progressReporter, CancellationToken cancellationToken) { CheckVersionValidity(version); diff --git a/src/Tgstation.Server.Host/Components/Engine/DelegatingEngineInstaller.cs b/src/Tgstation.Server.Host/Components/Engine/DelegatingEngineInstaller.cs index 91887e7314..c24eee3e8b 100644 --- a/src/Tgstation.Server.Host/Components/Engine/DelegatingEngineInstaller.cs +++ b/src/Tgstation.Server.Host/Components/Engine/DelegatingEngineInstaller.cs @@ -37,7 +37,7 @@ namespace Tgstation.Server.Host.Components.Engine => DelegateCall(version, installer => installer.CreateInstallation(version, path, installationTask)); /// - public ValueTask DownloadVersion(EngineVersion version, JobProgressReporter? jobProgressReporter, CancellationToken cancellationToken) + public ValueTask DownloadVersion(EngineVersion version, JobProgressReporter jobProgressReporter, CancellationToken cancellationToken) => DelegateCall(version, installer => installer.DownloadVersion(version, jobProgressReporter, cancellationToken)); /// diff --git a/src/Tgstation.Server.Host/Components/Engine/EngineInstallerBase.cs b/src/Tgstation.Server.Host/Components/Engine/EngineInstallerBase.cs index 12cf8e657c..6ca2b94030 100644 --- a/src/Tgstation.Server.Host/Components/Engine/EngineInstallerBase.cs +++ b/src/Tgstation.Server.Host/Components/Engine/EngineInstallerBase.cs @@ -52,7 +52,7 @@ namespace Tgstation.Server.Host.Components.Engine public abstract ValueTask UpgradeInstallation(EngineVersion version, string path, CancellationToken cancellationToken); /// - public abstract ValueTask DownloadVersion(EngineVersion version, JobProgressReporter? jobProgressReporter, CancellationToken cancellationToken); + public abstract ValueTask DownloadVersion(EngineVersion version, JobProgressReporter jobProgressReporter, CancellationToken cancellationToken); /// public abstract ValueTask TrustDmbPath(EngineVersion version, string fullDmbPath, CancellationToken cancellationToken); diff --git a/src/Tgstation.Server.Host/Components/Engine/EngineManager.cs b/src/Tgstation.Server.Host/Components/Engine/EngineManager.cs index f2a60ae5b9..970e3ab89d 100644 --- a/src/Tgstation.Server.Host/Components/Engine/EngineManager.cs +++ b/src/Tgstation.Server.Host/Components/Engine/EngineManager.cs @@ -118,7 +118,7 @@ namespace Tgstation.Server.Host.Components.Engine /// public async ValueTask ChangeVersion( - JobProgressReporter? progressReporter, + JobProgressReporter progressReporter, EngineVersion version, Stream? customVersionStream, bool allowInstallation, @@ -166,8 +166,11 @@ namespace Tgstation.Server.Host.Components.Engine "Acquiring lock on BYOND version {version}...", requiredVersion?.ToString() ?? $"{ActiveVersion} (active)"); var versionToUse = requiredVersion ?? ActiveVersion ?? throw new JobException(ErrorCode.EngineNoVersionsInstalled); + + using var progressReporter = new JobProgressReporter(); + var installLock = await AssertAndLockVersion( - null, + progressReporter, versionToUse, null, requiredVersion != null, @@ -388,7 +391,7 @@ namespace Tgstation.Server.Host.Components.Engine /// /// Ensures a BYOND is installed if it isn't already. /// - /// The optional for the operation. + /// The for the operation. /// The to install. /// Optional custom zip file to use. Will cause a number to be added. /// If this BYOND version is required as part of a locking operation. @@ -396,7 +399,7 @@ namespace Tgstation.Server.Host.Components.Engine /// The for the operation. /// A resulting in the . async ValueTask AssertAndLockVersion( - JobProgressReporter? progressReporter, + JobProgressReporter progressReporter, EngineVersion version, Stream? customVersionStream, bool neededForLock, @@ -443,8 +446,7 @@ namespace Tgstation.Server.Host.Components.Engine { if (installedOrInstalling) { - if (progressReporter != null) - progressReporter.StageName = "Waiting for existing installation job..."; + progressReporter.StageName = "Waiting for existing installation job..."; if (neededForLock && !installation.InstallationTask.IsCompleted) logger.LogWarning("The required engine version ({version}) is not readily available! We will have to wait for it to install.", version); @@ -468,8 +470,7 @@ namespace Tgstation.Server.Host.Components.Engine else logger.LogInformation("Requested engine version {version} not currently installed. Doing so now...", version); - if (progressReporter != null) - progressReporter.StageName = "Running event"; + progressReporter.StageName = "Running event"; var versionString = version.ToString(); await eventConsumer.HandleEvent(EventType.EngineInstallStart, new List { versionString }, deploymentPipelineProcesses, cancellationToken); @@ -504,14 +505,14 @@ namespace Tgstation.Server.Host.Components.Engine /// /// Installs the files for a given BYOND . /// - /// The optional for the operation. + /// The for the operation. /// The being installed with the number set if appropriate. /// Custom zip file to use. Will cause a number to be added. /// If processes should be launched as part of the deployment pipeline. /// The for the operation. /// A representing the running operation. async ValueTask InstallVersionFiles( - JobProgressReporter? progressReporter, + JobProgressReporter progressReporter, EngineVersion version, Stream? customVersionStream, bool deploymentPipelineProcesses, @@ -528,14 +529,12 @@ namespace Tgstation.Server.Host.Components.Engine try { IEngineInstallationData engineInstallationData; + var remainingProgress = 1.0; if (customVersionStream == null) { - if (progressReporter != null) - progressReporter.StageName = "Downloading version"; - - engineInstallationData = await engineInstaller.DownloadVersion(version, progressReporter, cancellationToken); - - progressReporter?.ReportProgress(null); + using var subReporter = progressReporter.CreateSection("Downloading Version", 0.5); + remainingProgress -= 0.5; + engineInstallationData = await engineInstaller.DownloadVersion(version, subReporter, cancellationToken); } else #pragma warning disable CA2000 // Dispose objects before losing scope, false positive @@ -544,33 +543,45 @@ namespace Tgstation.Server.Host.Components.Engine customVersionStream); #pragma warning restore CA2000 // Dispose objects before losing scope - await using (engineInstallationData) + JobProgressReporter remainingReporter; + try { - if (progressReporter != null) - progressReporter.StageName = "Cleaning target directory"; - - await directoryCleanupTask; - - if (progressReporter != null) - progressReporter.StageName = "Extracting data"; - - logger.LogTrace("Extracting engine to {extractPath}...", installFullPath); - await engineInstallationData.ExtractToPath(installFullPath, cancellationToken); + remainingReporter = progressReporter.CreateSection(null, remainingProgress); + } + catch + { + await engineInstallationData.DisposeAsync(); + throw; } - if (progressReporter != null) - progressReporter.StageName = "Running installation actions"; + using (remainingReporter) + { + await using (engineInstallationData) + { + remainingReporter.StageName = "Cleaning target directory"; - await engineInstaller.Install(version, installFullPath, deploymentPipelineProcesses, cancellationToken); + await directoryCleanupTask; + remainingReporter.ReportProgress(0.1); + remainingReporter.StageName = "Extracting data"; - if (progressReporter != null) - progressReporter.StageName = "Writing version file"; + logger.LogTrace("Extracting engine to {extractPath}...", installFullPath); + await engineInstallationData.ExtractToPath(installFullPath, cancellationToken); + remainingReporter.ReportProgress(0.3); + } - // make sure to do this last because this is what tells us we have a valid version in the future - await ioManager.WriteAllBytes( - ioManager.ConcatPath(installFullPath, VersionFileName), - Encoding.UTF8.GetBytes(version.ToString()), - cancellationToken); + remainingReporter.StageName = "Running installation actions"; + + await engineInstaller.Install(version, installFullPath, deploymentPipelineProcesses, cancellationToken); + + remainingReporter.ReportProgress(0.9); + remainingReporter.StageName = "Writing version file"; + + // make sure to do this last because this is what tells us we have a valid version in the future + await ioManager.WriteAllBytes( + ioManager.ConcatPath(installFullPath, VersionFileName), + Encoding.UTF8.GetBytes(version.ToString()), + cancellationToken); + } } catch (HttpRequestException ex) { diff --git a/src/Tgstation.Server.Host/Components/Engine/IEngineInstaller.cs b/src/Tgstation.Server.Host/Components/Engine/IEngineInstaller.cs index a4ad57a4c0..0df793e219 100644 --- a/src/Tgstation.Server.Host/Components/Engine/IEngineInstaller.cs +++ b/src/Tgstation.Server.Host/Components/Engine/IEngineInstaller.cs @@ -24,10 +24,10 @@ namespace Tgstation.Server.Host.Components.Engine /// Download a given engine . /// /// The of the engine to download. - /// The optional for the operation. + /// The for the operation. /// The for the operation. /// A resulting in the for the download. - ValueTask DownloadVersion(EngineVersion version, JobProgressReporter? jobProgressReporter, CancellationToken cancellationToken); + ValueTask DownloadVersion(EngineVersion version, JobProgressReporter jobProgressReporter, CancellationToken cancellationToken); /// /// Does actions necessary to get an extracted installation working. diff --git a/src/Tgstation.Server.Host/Components/Engine/IEngineManager.cs b/src/Tgstation.Server.Host/Components/Engine/IEngineManager.cs index 18af1469e6..7ce4b883db 100644 --- a/src/Tgstation.Server.Host/Components/Engine/IEngineManager.cs +++ b/src/Tgstation.Server.Host/Components/Engine/IEngineManager.cs @@ -28,14 +28,14 @@ namespace Tgstation.Server.Host.Components.Engine /// /// Change the active . /// - /// The optional for the operation. + /// The for the operation. /// The new . /// Optional of a custom BYOND version zip file. /// If an installation should be performed if the is not installed. If and an installation is required an will be thrown. /// The for the operation. /// A representing the running operation. ValueTask ChangeVersion( - JobProgressReporter? progressReporter, + JobProgressReporter progressReporter, EngineVersion version, Stream? customVersionStream, bool allowInstallation, diff --git a/src/Tgstation.Server.Host/Components/Engine/OpenDreamInstaller.cs b/src/Tgstation.Server.Host/Components/Engine/OpenDreamInstaller.cs index 608d3addb5..ac4c0964e9 100644 --- a/src/Tgstation.Server.Host/Components/Engine/OpenDreamInstaller.cs +++ b/src/Tgstation.Server.Host/Components/Engine/OpenDreamInstaller.cs @@ -133,23 +133,32 @@ namespace Tgstation.Server.Host.Components.Engine } /// - public override async ValueTask DownloadVersion(EngineVersion version, JobProgressReporter? jobProgressReporter, CancellationToken cancellationToken) + public override async ValueTask DownloadVersion(EngineVersion version, JobProgressReporter jobProgressReporter, CancellationToken cancellationToken) { CheckVersionValidity(version); + ArgumentNullException.ThrowIfNull(jobProgressReporter); // get a lock on a system wide OD repo Logger.LogTrace("Cloning OD repo..."); - var progressSection1 = jobProgressReporter?.CreateSection("Updating OpenDream git repository", 0.5f); - - var repo = await repositoryManager.CloneRepository( - GeneralConfiguration.OpenDreamGitUrl, - null, - null, - null, - progressSection1, - true, - cancellationToken); + var progressSection1 = jobProgressReporter.CreateSection("Updating OpenDream git repository", 0.5f); + IRepository? repo; + try + { + repo = await repositoryManager.CloneRepository( + GeneralConfiguration.OpenDreamGitUrl, + null, + null, + null, + progressSection1, + true, + cancellationToken); + } + catch + { + progressSection1.Dispose(); + throw; + } try { @@ -168,19 +177,23 @@ namespace Tgstation.Server.Host.Components.Engine cancellationToken); } - var progressSection2 = jobProgressReporter?.CreateSection("Checking out OpenDream version", 0.5f); + progressSection1.Dispose(); + progressSection1 = null; - var committish = version.SourceSHA - ?? $"{GeneralConfiguration.OpenDreamGitTagPrefix}{version.Version!.Semver()}"; + using (var progressSection2 = jobProgressReporter.CreateSection("Checking out OpenDream version", 0.5f)) + { + var committish = version.SourceSHA + ?? $"{GeneralConfiguration.OpenDreamGitTagPrefix}{version.Version!.Semver()}"; - await repo.CheckoutObject( - committish, - null, - null, - true, - false, - progressSection2, - cancellationToken); + await repo.CheckoutObject( + committish, + null, + null, + true, + false, + progressSection2, + cancellationToken); + } if (!await repo.CommittishIsParent("tgs-min-compat", cancellationToken)) throw new JobException(ErrorCode.OpenDreamTooOld); @@ -192,6 +205,10 @@ namespace Tgstation.Server.Host.Components.Engine repo?.Dispose(); throw; } + finally + { + progressSection1?.Dispose(); + } } /// diff --git a/src/Tgstation.Server.Host/Components/Repository/IRepository.cs b/src/Tgstation.Server.Host/Components/Repository/IRepository.cs index e736c4873e..74417818f1 100644 --- a/src/Tgstation.Server.Host/Components/Repository/IRepository.cs +++ b/src/Tgstation.Server.Host/Components/Repository/IRepository.cs @@ -48,7 +48,7 @@ namespace Tgstation.Server.Host.Components.Repository /// The optional password used for fetching from submodule repositories. /// If a submodule update should be attempted after the merge. /// If a hard reset to the target committish should be performed instead of a checkout. - /// The optional to report progress of the operation. + /// The to report progress of the operation. /// The for the operation. /// A representing the running operation. ValueTask CheckoutObject( @@ -57,7 +57,7 @@ namespace Tgstation.Server.Host.Components.Repository string? password, bool updateSubmodules, bool moveCurrentReference, - JobProgressReporter? progressReporter, + JobProgressReporter progressReporter, CancellationToken cancellationToken); /// @@ -85,14 +85,14 @@ namespace Tgstation.Server.Host.Components.Repository /// /// Fetch commits from the origin repository. /// - /// The optional to report progress of the operation. + /// The to report progress of the operation. /// The optional username to fetch from the origin repository. /// The optional password to fetch from the origin repository. /// If any events created should be marked as part of the deployment pipeline. /// The for the operation. /// A representing the running operation. ValueTask FetchOrigin( - JobProgressReporter? progressReporter, + JobProgressReporter progressReporter, string? username, string? password, bool deploymentPipeline, diff --git a/src/Tgstation.Server.Host/Components/Repository/IRepositoryManager.cs b/src/Tgstation.Server.Host/Components/Repository/IRepositoryManager.cs index 596301611a..87cbe63d95 100644 --- a/src/Tgstation.Server.Host/Components/Repository/IRepositoryManager.cs +++ b/src/Tgstation.Server.Host/Components/Repository/IRepositoryManager.cs @@ -35,7 +35,7 @@ namespace Tgstation.Server.Host.Components.Repository /// The optional branch to clone. /// The optional username to clone from . /// The optional password to clone from . - /// The optional for progress of the clone. + /// The for progress of the clone. /// If submodules should be recusively cloned and initialized. /// The for the operation. /// A resulting i the newly cloned , if one already exists. @@ -44,7 +44,7 @@ namespace Tgstation.Server.Host.Components.Repository string? initialBranch, string? username, string? password, - JobProgressReporter? progressReporter, + JobProgressReporter progressReporter, bool recurseSubmodules, CancellationToken cancellationToken); diff --git a/src/Tgstation.Server.Host/Components/Repository/Repository.cs b/src/Tgstation.Server.Host/Components/Repository/Repository.cs index 6873d42957..9a71bcdceb 100644 --- a/src/Tgstation.Server.Host/Components/Repository/Repository.cs +++ b/src/Tgstation.Server.Host/Components/Repository/Repository.cs @@ -232,13 +232,14 @@ namespace Tgstation.Server.Host.Components.Repository logger.LogTrace("Fetching refspec {refSpec}...", refSpec); var remote = libGitRepo.Network.Remotes.First(); + using var fetchReporter = progressReporter.CreateSection($"Fetch {refSpec}", progressFactor); commands.Fetch( libGitRepo, refSpecList, remote, new FetchOptions().Hydrate( logger, - progressReporter.CreateSection($"Fetch {refSpec}", progressFactor), + fetchReporter, credentialsProvider.GenerateCredentialsHandler(username, password), cancellationToken), logMessage); @@ -267,14 +268,14 @@ namespace Tgstation.Server.Host.Components.Repository logger.LogTrace("Merging {targetCommitSha} into {currentReference}...", testMergeParameters.TargetCommitSha[..7], Reference); + using var mergeReporter = progressReporter.CreateSection($"Merge {testMergeParameters.TargetCommitSha[..7]}", progressFactor); result = libGitRepo.Merge(testMergeParameters.TargetCommitSha, sig, new MergeOptions { CommitOnSuccess = commitMessage == null, FailOnConflict = false, // Needed to get conflicting files FastForwardStrategy = FastForwardStrategy.NoFastForward, SkipReuc = true, - OnCheckoutProgress = CheckoutProgressHandler( - progressReporter.CreateSection($"Merge {testMergeParameters.TargetCommitSha[..7]}", progressFactor)), + OnCheckoutProgress = CheckoutProgressHandler(mergeReporter), }); } finally @@ -295,7 +296,8 @@ namespace Tgstation.Server.Host.Components.Repository var revertTo = originalCommit.CanonicalName ?? originalCommit.Tip.Sha; logger.LogDebug("Merge conflict, aborting and reverting to {revertTarget}", revertTo); progressReporter.ReportProgress(0); - RawCheckout(revertTo, false, progressReporter.CreateSection("Hard Reset to {revertTo}", 1.0), cancellationToken); + using var revertReporter = progressReporter.CreateSection("Hard Reset to {revertTo}", 1.0); + RawCheckout(revertTo, false, revertReporter, cancellationToken); cancellationToken.ThrowIfCancellationRequested(); } @@ -343,8 +345,9 @@ namespace Tgstation.Server.Host.Components.Repository if (updateSubmodules) { + using var progressReporter2 = progressReporter.CreateSection("Update Submodules", progressFactor); await UpdateSubmodules( - progressReporter.CreateSection("Update Submodules", progressFactor), + progressReporter2, username, password, false, @@ -377,7 +380,7 @@ namespace Tgstation.Server.Host.Components.Repository string? password, bool updateSubmodules, bool moveCurrentReference, - JobProgressReporter? progressReporter, + JobProgressReporter progressReporter, CancellationToken cancellationToken) { ArgumentNullException.ThrowIfNull(committish); @@ -388,10 +391,11 @@ namespace Tgstation.Server.Host.Components.Repository () => { libGitRepo.RemoveUntrackedFiles(); + using var progressReporter3 = progressReporter.CreateSection(null, updateSubmodules ? 2.0 / 3 : 1.0); RawCheckout( committish, moveCurrentReference, - progressReporter?.CreateSection(null, updateSubmodules ? 2.0 / 3 : 1.0), + progressReporter3, cancellationToken); }, cancellationToken, @@ -399,17 +403,20 @@ namespace Tgstation.Server.Host.Components.Repository TaskScheduler.Current); if (updateSubmodules) + { + using var progressReporter2 = progressReporter.CreateSection(null, 1.0 / 3); await UpdateSubmodules( - progressReporter?.CreateSection(null, 1.0 / 3), + progressReporter2, username, password, false, cancellationToken); + } } /// public async ValueTask FetchOrigin( - JobProgressReporter? progressReporter, + JobProgressReporter progressReporter, string? username, string? password, bool deploymentPipeline, @@ -423,13 +430,14 @@ namespace Tgstation.Server.Host.Components.Repository var remote = libGitRepo.Network.Remotes.First(); try { + using var subReporter = progressReporter.CreateSection("Fetch Origin", 1.0); var fetchOptions = new FetchOptions { Prune = true, TagFetchMode = TagFetchMode.All, }.Hydrate( logger, - progressReporter?.CreateSection("Fetch Origin", 1.0), + subReporter, credentialsProvider.GenerateCredentialsHandler(username, password), cancellationToken); @@ -471,18 +479,23 @@ namespace Tgstation.Server.Host.Components.Repository logger.LogTrace("Reset to origin..."); var trackedBranch = libGitRepo.Head.TrackedBranch; await eventConsumer.HandleEvent(EventType.RepoResetOrigin, new List { trackedBranch.FriendlyName, trackedBranch.Tip.Sha }, deploymentPipeline, cancellationToken); - await ResetToSha( - trackedBranch.Tip.Sha, - progressReporter.CreateSection(null, updateSubmodules ? 2.0 / 3 : 1.0), - cancellationToken); + + using (var progressReporter2 = progressReporter.CreateSection(null, updateSubmodules ? 2.0 / 3 : 1.0)) + await ResetToSha( + trackedBranch.Tip.Sha, + progressReporter2, + cancellationToken); if (updateSubmodules) + { + using var progressReporter3 = progressReporter.CreateSection(null, 1.0 / 3); await UpdateSubmodules( - progressReporter.CreateSection(null, 1.0 / 3), + progressReporter3, username, password, deploymentPipeline, cancellationToken); + } } /// @@ -684,9 +697,10 @@ namespace Tgstation.Server.Host.Components.Repository await Task.Factory.StartNew( () => { + using var resetProgress = progressReporter.CreateSection("Hard reset and remove untracked files", 0.1); libGitRepo.Reset(ResetMode.Hard, libGitRepo.Head.Tip, new CheckoutOptions { - OnCheckoutProgress = CheckoutProgressHandler(progressReporter.CreateSection("Hard reset and remove untracked files", 0.1)), + OnCheckoutProgress = CheckoutProgressHandler(resetProgress), }); cancellationToken.ThrowIfCancellationRequested(); libGitRepo.RemoveUntrackedFiles(); @@ -699,10 +713,11 @@ namespace Tgstation.Server.Host.Components.Repository var remainingProgressFactor = 0.9; if (!synchronizeTrackedBranch) { + using var progressReporter2 = progressReporter.CreateSection("Push to temporary branch", remainingProgressFactor); await PushHeadToTemporaryBranch( username, password, - progressReporter.CreateSection("Push to temporary branch", remainingProgressFactor), + progressReporter2, cancellationToken); return false; } @@ -722,13 +737,24 @@ namespace Tgstation.Server.Host.Components.Repository var remote = libGitRepo.Network.Remotes.First(); try { - libGitRepo.Network.Push( - libGitRepo.Head, - GeneratePushOptions( - progressReporter.CreateSection("Push to origin", remainingProgressFactor), - username, - password, - cancellationToken)); + using var pushReporter = progressReporter.CreateSection("Push to origin", remainingProgressFactor); + var (pushOptions, progressReporters) = GeneratePushOptions( + pushReporter, + username, + password, + cancellationToken); + try + { + libGitRepo.Network.Push( + libGitRepo.Head, + pushOptions); + } + finally + { + foreach (var progressReporter in progressReporters) + progressReporter.Dispose(); + } + return true; } catch (NonFastForwardException) @@ -882,9 +908,9 @@ namespace Tgstation.Server.Host.Components.Repository /// /// The committish to checkout. /// If a hard reset should actually be performed. - /// The optional for the operation. + /// The for the operation. /// The for the operation. - void RawCheckout(string committish, bool moveCurrentReference, JobProgressReporter? progressReporter, CancellationToken cancellationToken) + void RawCheckout(string committish, bool moveCurrentReference, JobProgressReporter progressReporter, CancellationToken cancellationToken) { logger.LogTrace("Checkout: {committish}", committish); @@ -893,13 +919,10 @@ namespace Tgstation.Server.Host.Components.Repository CheckoutModifiers = CheckoutModifiers.Force, }; - if (progressReporter != null) - { - var stage = $"Checkout {committish}"; - progressReporter = progressReporter.CreateSection(stage, 1.0); - progressReporter.ReportProgress(0); - checkoutOptions.OnCheckoutProgress = CheckoutProgressHandler(progressReporter); - } + var stage = $"Checkout {committish}"; + using var newProgressReporter = progressReporter.CreateSection(stage, 1.0); + newProgressReporter.ReportProgress(0); + checkoutOptions.OnCheckoutProgress = CheckoutProgressHandler(newProgressReporter); cancellationToken.ThrowIfCancellationRequested(); @@ -976,9 +999,38 @@ namespace Tgstation.Server.Host.Components.Repository try { var forcePushString = String.Format(CultureInfo.InvariantCulture, "+{0}:{0}", branch.CanonicalName); - libGitRepo.Network.Push(remote, forcePushString, GeneratePushOptions(progressReporter.CreateSection(null, 0.9), username, password, cancellationToken)); + + using (var mainPushReporter = progressReporter.CreateSection(null, 0.9)) + { + var (pushOptions, progressReporters) = GeneratePushOptions( + mainPushReporter, + username, + password, + cancellationToken); + + try + { + libGitRepo.Network.Push(remote, forcePushString, pushOptions); + } + finally + { + foreach (var progressReporter in progressReporters) + progressReporter.Dispose(); + } + } + var removalString = String.Format(CultureInfo.InvariantCulture, ":{0}", branch.CanonicalName); - libGitRepo.Network.Push(remote, removalString, GeneratePushOptions(progressReporter.CreateSection(null, 0.1), username, password, cancellationToken)); + using var forcePushReporter = progressReporter.CreateSection(null, 0.1); + var (forcePushOptions, forcePushReporters) = GeneratePushOptions(forcePushReporter, username, password, cancellationToken); + try + { + libGitRepo.Network.Push(remote, removalString, forcePushOptions); + } + finally + { + foreach (var subForcePushReporter in forcePushReporters) + forcePushReporter.Dispose(); + } } catch (UserCancelledException) { @@ -1005,32 +1057,39 @@ namespace Tgstation.Server.Host.Components.Repository /// The username for the . /// The password for the . /// The for the operation. - /// A new set of . - PushOptions GeneratePushOptions(JobProgressReporter progressReporter, string username, string password, CancellationToken cancellationToken) + /// A new set of and the associated s based off . + (PushOptions PushOptions, IEnumerable SubProgressReporters) GeneratePushOptions(JobProgressReporter progressReporter, string username, string password, CancellationToken cancellationToken) { - var subProgressReporter = progressReporter.CreateSection(null, 0.5); + var packFileCountingReporter = progressReporter.CreateSection(null, 0.25); + var packFileDeltafyingReporter = progressReporter.CreateSection(null, 0.25); + var transferProgressReporter = progressReporter.CreateSection(null, 0.5); - return new PushOptions - { - OnPackBuilderProgress = (stage, current, total) => + return ( + PushOptions: new PushOptions { - var baseProgress = stage == PackBuilderStage.Counting ? 0 : 0.5; - var addon = total > 0 && current <= total ? (0.5 * ((double)current / total)) : 0; - progressReporter.ReportProgress(baseProgress + addon); - return !cancellationToken.IsCancellationRequested; + OnPackBuilderProgress = (stage, current, total) => + { + if (total < current) + total = current; + + var percentage = ((double)current) / total; + (stage == PackBuilderStage.Counting ? packFileCountingReporter : packFileDeltafyingReporter).ReportProgress(percentage); + return !cancellationToken.IsCancellationRequested; + }, + OnNegotiationCompletedBeforePush = (a) => !cancellationToken.IsCancellationRequested, + OnPushTransferProgress = (a, sentBytes, totalBytes) => + { + packFileCountingReporter.ReportProgress((double)sentBytes / totalBytes); + return !cancellationToken.IsCancellationRequested; + }, + CredentialsProvider = credentialsProvider.GenerateCredentialsHandler(username, password), }, - OnNegotiationCompletedBeforePush = (a) => + SubProgressReporters: new List { - subProgressReporter = progressReporter.CreateSection(null, 0.5); - return !cancellationToken.IsCancellationRequested; - }, - OnPushTransferProgress = (a, sentBytes, totalBytes) => - { - progressReporter.ReportProgress((double)sentBytes / totalBytes); - return !cancellationToken.IsCancellationRequested; - }, - CredentialsProvider = credentialsProvider.GenerateCredentialsHandler(username, password), - }; + packFileCountingReporter, + packFileDeltafyingReporter, + transferProgressReporter, + }); } /// @@ -1054,7 +1113,7 @@ namespace Tgstation.Server.Host.Components.Repository /// The for the operation. /// A representing the running operation. ValueTask UpdateSubmodules( - JobProgressReporter? progressReporter, + JobProgressReporter progressReporter, string? username, string? password, bool deploymentPipeline, @@ -1062,7 +1121,7 @@ namespace Tgstation.Server.Host.Components.Repository { logger.LogTrace("Updating submodules {withOrWithout} credentials...", username == null ? "without" : "with"); - async ValueTask RecursiveUpdateSubmodules(LibGit2Sharp.IRepository parentRepository, JobProgressReporter? currentProgressReporter, string parentGitDirectory) + async ValueTask RecursiveUpdateSubmodules(LibGit2Sharp.IRepository parentRepository, JobProgressReporter currentProgressReporter, string parentGitDirectory) { var submoduleCount = libGitRepo.Submodules.Count(); if (submoduleCount == 0) @@ -1081,15 +1140,16 @@ namespace Tgstation.Server.Host.Components.Repository OnCheckoutNotify = (_, _) => !cancellationToken.IsCancellationRequested, }; + using var fetchReporter = currentProgressReporter.CreateSection($"Fetch submodule {submodule.Name}", factor); + submoduleUpdateOptions.FetchOptions.Hydrate( logger, - currentProgressReporter?.CreateSection($"Fetch submodule {submodule.Name}", factor), + fetchReporter, credentialsProvider.GenerateCredentialsHandler(username, password), cancellationToken); - if (currentProgressReporter != null) - submoduleUpdateOptions.OnCheckoutProgress = CheckoutProgressHandler( - currentProgressReporter.CreateSection($"Checkout submodule {submodule.Name}", factor)); + using var checkoutReporter = currentProgressReporter.CreateSection($"Checkout submodule {submodule.Name}", factor); + submoduleUpdateOptions.OnCheckoutProgress = CheckoutProgressHandler(checkoutReporter); logger.LogDebug("Updating submodule {submoduleName}...", submodule.Name); Task RawSubModuleUpdate() => Task.Factory.StartNew( @@ -1106,7 +1166,7 @@ namespace Tgstation.Server.Host.Components.Repository { // workaround for https://github.com/libgit2/libgit2/issues/3820 // kill off the modules/ folder in .git and try again - currentProgressReporter?.ReportProgress(null); + currentProgressReporter.ReportProgress(0); credentialsProvider.CheckBadCredentialsException(ex); logger.LogWarning(ex, "Initial update of submodule {submoduleName} failed. Deleting submodule directories and re-attempting...", submodule.Name); @@ -1145,9 +1205,11 @@ namespace Tgstation.Server.Host.Components.Repository using var submoduleRepo = await submoduleFactory.CreateFromPath( submodulePath, cancellationToken); + + using var submoduleReporter = currentProgressReporter.CreateSection($"Entering submodule \"{submodule.Name}\"...", factor); await RecursiveUpdateSubmodules( submoduleRepo, - currentProgressReporter?.CreateSection($"Entering submodule \"{submodule.Name}\"...", factor), + submoduleReporter, submodulePath); } } diff --git a/src/Tgstation.Server.Host/Components/Repository/RepositoryManager.cs b/src/Tgstation.Server.Host/Components/Repository/RepositoryManager.cs index 94ae176951..74a6bc6751 100644 --- a/src/Tgstation.Server.Host/Components/Repository/RepositoryManager.cs +++ b/src/Tgstation.Server.Host/Components/Repository/RepositoryManager.cs @@ -123,7 +123,7 @@ namespace Tgstation.Server.Host.Components.Repository string? initialBranch, string? username, string? password, - JobProgressReporter? progressReporter, + JobProgressReporter progressReporter, bool recurseSubmodules, CancellationToken cancellationToken) { @@ -146,8 +146,8 @@ namespace Tgstation.Server.Host.Components.Repository if (!await ioManager.DirectoryExists(repositoryPath, cancellationToken)) try { - var cloneProgressReporter = progressReporter?.CreateSection(null, 0.75f); - var checkoutProgressReporter = progressReporter?.CreateSection(null, 0.25f); + using var cloneProgressReporter = progressReporter.CreateSection(null, 0.75f); + using var checkoutProgressReporter = progressReporter.CreateSection(null, 0.25f); var cloneOptions = new CloneOptions { RecurseSubmodules = recurseSubmodules, diff --git a/src/Tgstation.Server.Host/Components/Repository/RepositoryUpdateService.cs b/src/Tgstation.Server.Host/Components/Repository/RepositoryUpdateService.cs index 9861a85418..17e820433a 100644 --- a/src/Tgstation.Server.Host/Components/Repository/RepositoryUpdateService.cs +++ b/src/Tgstation.Server.Host/Components/Repository/RepositoryUpdateService.cs @@ -173,10 +173,7 @@ namespace Tgstation.Server.Host.Components.Repository var numSteps = (model.NewTestMerges?.Count ?? 0) + (model.UpdateFromOrigin == true ? 1 : 0) + (!modelHasShaOrReference ? 2 : (hardResettingToOriginReference ? 3 : 1)); var progressFactor = 1.0 / numSteps; - JobProgressReporter NextProgressReporter(string? stage) - { - return progressReporter.CreateSection(stage, progressFactor); - } + JobProgressReporter NextProgressReporter(string? stage) => progressReporter.CreateSection(stage, progressFactor); progressReporter.ReportProgress(0); @@ -246,29 +243,35 @@ namespace Tgstation.Server.Host.Components.Repository { if (!repo.Tracking) throw new JobException(ErrorCode.RepoReferenceRequired); - await repo.FetchOrigin( - NextProgressReporter("Fetch Origin"), - currentModel.AccessUser, - currentModel.AccessToken, - false, - cancellationToken); + using (var fetchReporter = NextProgressReporter("Fetch Origin")) + await repo.FetchOrigin( + fetchReporter, + currentModel.AccessUser, + currentModel.AccessToken, + false, + cancellationToken); if (!modelHasShaOrReference) { - var fastForward = await repo.MergeOrigin( - NextProgressReporter("Merge Origin"), - committerName, - currentModel.CommitterEmail!, - false, - cancellationToken); + bool? fastForward; + using (var mergeReporter = NextProgressReporter("Merge Origin")) + fastForward = await repo.MergeOrigin( + mergeReporter, + committerName, + currentModel.CommitterEmail!, + false, + cancellationToken); + if (!fastForward.HasValue) throw new JobException(ErrorCode.RepoMergeConflict); + lastRevisionInfo!.OriginCommitSha = await repo.GetOriginSha(cancellationToken); await UpdateRevInfo(); if (fastForward.Value) { + using var syncReporter = NextProgressReporter("Sychronize"); await repo.Synchronize( - NextProgressReporter("Sychronize"), + syncReporter, currentModel.AccessUser, currentModel.AccessToken, currentModel.CommitterName!, @@ -279,7 +282,7 @@ namespace Tgstation.Server.Host.Components.Repository postUpdateSha = repo.Head; } else - NextProgressReporter(null).ReportProgress(1.0); + NextProgressReporter(null).Dispose(); } } @@ -303,39 +306,44 @@ namespace Tgstation.Server.Host.Components.Repository if ((isSha && model.Reference != null) || (!isSha && model.CheckoutSha != null)) throw new JobException(ErrorCode.RepoSwappedShaOrReference); - await repo.CheckoutObject( - committish, - currentModel.AccessUser, - currentModel.AccessToken, - updateSubmodules, - false, - NextProgressReporter("Checkout"), - cancellationToken); + using (var checkoutReporter = NextProgressReporter("Checkout")) + await repo.CheckoutObject( + committish, + currentModel.AccessUser, + currentModel.AccessToken, + updateSubmodules, + false, + checkoutReporter, + cancellationToken); await CallLoadRevInfo(); // we've either seen origin before or what we're checking out is on origin } else - NextProgressReporter(null).ReportProgress(1.0); + NextProgressReporter(null).Dispose(); if (hardResettingToOriginReference) { if (!repo.Tracking) throw new JobException(ErrorCode.RepoReferenceNotTracking); - await repo.ResetToOrigin( - NextProgressReporter("Reset to Origin"), - currentModel.AccessUser, - currentModel.AccessToken, - updateSubmodules, - false, - cancellationToken); - await repo.Synchronize( - NextProgressReporter("Synchronize"), - currentModel.AccessUser, - currentModel.AccessToken, - currentModel.CommitterName!, - currentModel.CommitterEmail!, - true, - false, - cancellationToken); + using (var resetReporter = NextProgressReporter("Reset to Origin")) + await repo.ResetToOrigin( + resetReporter, + currentModel.AccessUser, + currentModel.AccessToken, + updateSubmodules, + false, + cancellationToken); + + using (var syncReporter = NextProgressReporter("Synchronize")) + await repo.Synchronize( + syncReporter, + currentModel.AccessUser, + currentModel.AccessToken, + currentModel.CommitterName!, + currentModel.CommitterEmail!, + true, + false, + cancellationToken); + await CallLoadRevInfo(); // repo head is on origin so force this @@ -486,7 +494,8 @@ namespace Tgstation.Server.Host.Components.Repository // goteem var commitSha = revInfoWereLookingFor.CommitSha!; logger.LogDebug("Reusing existing SHA {sha}...", commitSha); - await repo.ResetToSha(commitSha, NextProgressReporter($"Reset to {commitSha[..7]}"), cancellationToken); + using var resetReporter = NextProgressReporter($"Reset to {commitSha[..7]}"); + await repo.ResetToSha(commitSha, resetReporter, cancellationToken); lastRevisionInfo = revInfoWereLookingFor; } @@ -499,15 +508,17 @@ namespace Tgstation.Server.Host.Components.Repository var fullTestMergeTask = repo.GetTestMerge(newTestMerge, currentModel, cancellationToken); - var mergeResult = await repo.AddTestMerge( - newTestMerge, - committerName, - currentModel.CommitterEmail!, - currentModel.AccessUser, - currentModel.AccessToken, - updateSubmodules, - NextProgressReporter($"Test merge #{newTestMerge.Number}"), - cancellationToken); + TestMergeResult mergeResult; + using (var testMergeReporter = NextProgressReporter($"Test merge #{newTestMerge.Number}")) + mergeResult = await repo.AddTestMerge( + newTestMerge, + committerName, + currentModel.CommitterEmail!, + currentModel.AccessUser, + currentModel.AccessToken, + updateSubmodules, + testMergeReporter, + cancellationToken); if (mergeResult.Status == MergeStatus.Conflicts) throw new JobException( @@ -546,15 +557,17 @@ namespace Tgstation.Server.Host.Components.Repository var currentHead = repo.Head; if (currentModel.PushTestMergeCommits!.Value && (startSha != currentHead || (postUpdateSha != null && postUpdateSha != currentHead))) { - await repo.Synchronize( - NextProgressReporter("Synchronize"), - currentModel.AccessUser, - currentModel.AccessToken, - currentModel.CommitterName!, - currentModel.CommitterEmail!, - false, - false, - cancellationToken); + using (var syncReporter = NextProgressReporter("Synchronize")) + await repo.Synchronize( + syncReporter, + currentModel.AccessUser, + currentModel.AccessToken, + currentModel.CommitterName!, + currentModel.CommitterEmail!, + false, + false, + cancellationToken); + await UpdateRevInfo(); } } @@ -568,17 +581,19 @@ namespace Tgstation.Server.Host.Components.Repository var secondStep = startReference != null && repo.Head != startSha; // DCTx2: Cancellation token is for job, operations should always run - await repo.CheckoutObject( - startReference ?? startSha, - currentModel.AccessUser, - currentModel.AccessToken, - true, - false, - progressReporter.CreateSection($"Checkout {startReference ?? startSha[..7]}", secondStep ? 0.5 : 1.0), - default); + using (var checkoutReporter = progressReporter.CreateSection($"Checkout {startReference ?? startSha[..7]}", secondStep ? 0.5 : 1.0)) + await repo.CheckoutObject( + startReference ?? startSha, + currentModel.AccessUser, + currentModel.AccessToken, + true, + false, + checkoutReporter, + default); if (secondStep) - await repo.ResetToSha(startSha, progressReporter.CreateSection($"Hard reset to SHA {startSha[..7]}", 0.5), default); + using (var resetReporter = progressReporter.CreateSection($"Hard reset to SHA {startSha[..7]}", 0.5)) + await repo.ResetToSha(startSha, resetReporter, default); throw; } @@ -608,32 +623,35 @@ namespace Tgstation.Server.Host.Components.Repository string? oldReference; string oldSha; ValueTask deleteTask; - using (var oldRepo = await instance.RepositoryManager.LoadRepository(cancellationToken)) + using (var deleteReporter = progressReporter.CreateSection("Deleting Old Repository", 0.1)) { - if (oldRepo == null) - throw new JobException(ErrorCode.RepoMissing); + using (var oldRepo = await instance.RepositoryManager.LoadRepository(cancellationToken)) + { + if (oldRepo == null) + throw new JobException(ErrorCode.RepoMissing); - origin = oldRepo.Origin; - oldSha = oldRepo.Head; - oldReference = oldRepo.Reference; - if (oldReference == Repository.NoReference) - oldReference = null; + origin = oldRepo.Origin; + oldSha = oldRepo.Head; + oldReference = oldRepo.Reference; + if (oldReference == Repository.NoReference) + oldReference = null; - progressReporter.StageName = "Deleting Old Repository"; - deleteTask = instance.RepositoryManager.DeleteRepository(cancellationToken); + deleteTask = instance.RepositoryManager.DeleteRepository(cancellationToken); + } + + await deleteTask; } - await deleteTask; - progressReporter.ReportProgress(0.1); IRepository newRepo; try { + using var cloneReporter = progressReporter.CreateSection("Cloning New Repository", 0.8); newRepo = await instance.RepositoryManager.CloneRepository( origin, oldReference, currentModel.AccessUser, currentModel.AccessToken, - progressReporter.CreateSection("Cloning New Repository", 0.8), + cloneReporter, true, // TODO: Make configurable maybe... cancellationToken) ?? throw new JobException("A race condition occurred while recloning the repository. Somehow, it was fully cloned instantly after being deleted!"); // I'll take lines of code that should never be hit for $10k @@ -655,14 +673,17 @@ namespace Tgstation.Server.Host.Components.Repository } using (newRepo) + using (var checkoutReporter = progressReporter.CreateSection("Checking out previous Detached Commit", 0.1)) + { await newRepo.CheckoutObject( oldSha, currentModel.AccessUser, currentModel.AccessToken, false, oldReference != null, - progressReporter.CreateSection("Checking out previous Detached Commit", 0.1), + checkoutReporter, cancellationToken); + } } } } diff --git a/src/Tgstation.Server.Host/Components/Session/SessionController.cs b/src/Tgstation.Server.Host/Components/Session/SessionController.cs index 6880ed7f6d..754b672b7d 100644 --- a/src/Tgstation.Server.Host/Components/Session/SessionController.cs +++ b/src/Tgstation.Server.Host/Components/Session/SessionController.cs @@ -122,7 +122,7 @@ namespace Tgstation.Server.Host.Components.Session public FifoSemaphore TopicSendSemaphore { get; } /// - public long MemoryUsage => process.MemoryUsage; + public long? MemoryUsage => process.MemoryUsage; /// /// The for the . diff --git a/src/Tgstation.Server.Host/Controllers/EngineController.cs b/src/Tgstation.Server.Host/Controllers/EngineController.cs index c1a20fe99c..83a113e3b4 100644 --- a/src/Tgstation.Server.Host/Controllers/EngineController.cs +++ b/src/Tgstation.Server.Host/Controllers/EngineController.cs @@ -184,7 +184,8 @@ namespace Tgstation.Server.Host.Controllers try { - await byondManager.ChangeVersion(null, model.EngineVersion, null, false, cancellationToken); + using var progressReporter = new JobProgressReporter(); + await byondManager.ChangeVersion(progressReporter, model.EngineVersion, null, false, cancellationToken); } catch (InvalidOperationException ex) { diff --git a/src/Tgstation.Server.Host/Extensions/FetchOptionsExtensions.cs b/src/Tgstation.Server.Host/Extensions/FetchOptionsExtensions.cs index 6d71a68edf..b5e996c311 100644 --- a/src/Tgstation.Server.Host/Extensions/FetchOptionsExtensions.cs +++ b/src/Tgstation.Server.Host/Extensions/FetchOptionsExtensions.cs @@ -20,14 +20,14 @@ namespace Tgstation.Server.Host.Extensions /// /// The to hydrate. /// The for the operation. - /// The optional . + /// The . /// The optional . /// The for the operation. /// The hydrated . public static FetchOptions Hydrate( this FetchOptions fetchOptions, ILogger logger, - JobProgressReporter? progressReporter, + JobProgressReporter progressReporter, CredentialsHandler credentialsHandler, CancellationToken cancellationToken) { @@ -60,10 +60,10 @@ namespace Tgstation.Server.Host.Extensions /// Generate a from a given and . /// /// The for the operation. - /// The optional of the operation. + /// The of the operation. /// The for the operation. /// A new based on . - static TransferProgressHandler TransferProgressHandler(ILogger logger, JobProgressReporter? progressReporter, CancellationToken cancellationToken) => transferProgress => + static TransferProgressHandler TransferProgressHandler(ILogger logger, JobProgressReporter progressReporter, CancellationToken cancellationToken) => transferProgress => { double? percentage; var totalObjectsToProcess = transferProgress.TotalObjects * 2; diff --git a/src/Tgstation.Server.Host/Jobs/JobProgressReporter.cs b/src/Tgstation.Server.Host/Jobs/JobProgressReporter.cs index 753f8cf519..12ee246858 100644 --- a/src/Tgstation.Server.Host/Jobs/JobProgressReporter.cs +++ b/src/Tgstation.Server.Host/Jobs/JobProgressReporter.cs @@ -1,6 +1,7 @@ using System; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using Tgstation.Server.Host.Models; @@ -9,7 +10,7 @@ namespace Tgstation.Server.Host.Jobs /// /// Progress reporter for a . /// - public sealed class JobProgressReporter + public sealed class JobProgressReporter : IDisposable { /// /// The name of the current stage. @@ -52,6 +53,24 @@ namespace Tgstation.Server.Host.Jobs /// double sectionProgression; + /// + /// The total progress reserved for use in this section. + /// + double? sectionReservations; + + /// + /// Initializes a new instance of the class. + /// + /// This variant has no function. + public JobProgressReporter() + : this( + NullLogger.Instance, + null, + (_, _) => { }, + false) + { + } + /// /// Initializes a new instance of the class. /// @@ -59,27 +78,84 @@ namespace Tgstation.Server.Host.Jobs /// The value of . /// The value of . public JobProgressReporter(ILogger logger, string? stageName, Action callback) + : this( + logger, + stageName, + callback, + true) + { + } + + /// + /// Initializes a new instance of the class. + /// + /// The value of . + /// The value of . + /// The value of . + /// If an initial call to will be made with only the . + private JobProgressReporter(ILogger logger, string? stageName, Action callback, bool setStageName) { this.logger = logger ?? throw new ArgumentNullException(nameof(logger)); this.callback = callback ?? throw new ArgumentNullException(nameof(callback)); - StageName = stageName; + if (setStageName) + { + StageName = stageName; + } + else + { + this.stageName = stageName; + } logger.LogDebug("Job progress reporter created. Stage: {stageName}", stageName ?? "(null)"); } + /// + public void Dispose() + { + if (sectionReservations.HasValue) + if (sectionReservations.Value != 1.0) + { + // not an error, processes can throw + sectionReservations = null; + } + else if (sectionProgression < 1.0) + { + logger.LogError( + new InvalidOperationException($"Parent progress reporter has child sections that didn't complete! Current: {sectionProgression}"), + "TGS BUG: Progress reporter children didn't complete!"); + sectionReservations = null; + } + + if (!sectionReservations.HasValue) + ReportProgress(1); + } + /// /// Report progress. /// /// A percentage value from 0.0f-1.0f. public void ReportProgress(double? progress) { + if (sectionReservations.HasValue) + if (progress == 0) + { + // might be a stage reset + sectionReservations = null; + } + else + { + logger.LogError( + new InvalidOperationException("Progress reporter is reporting progress with existing nested sections!"), + "TGS BUG: A progress reporter is using mixed local and nested progress, this is not supported"); + } + var clampedProgress = progress; if (progress.HasValue) if (progress > 1 || progress < 0) { logger.LogError( new ArgumentOutOfRangeException(nameof(progress), progress, "Progress must be a value from 0-1!"), - "Invalid progress value for stage {stageName}", + "TGS BUG: Invalid progress value for stage {stageName}", StageName ?? "(null)"); clampedProgress = null; } @@ -103,16 +179,28 @@ namespace Tgstation.Server.Host.Jobs { logger.LogError( new ArgumentOutOfRangeException(nameof(percentage), percentage, "Percentage must be a value from 0-1!"), - "Invalid percentage value for stage {newStageName}! Clamping...", + "TGS BUG: Invalid percentage value for stage {newStageName}! Clamping...", newStageName ?? "(null)"); percentage = Math.Min(Math.Max(percentage, 0.0), 1.0); } - var childBaseProgress = sectionProgression; - if (percentage + childBaseProgress > 1.0) + if (!sectionReservations.HasValue) { - var remainingPercentage = 1.0 - childBaseProgress; + if (sectionProgression != 0) + { + logger.LogError( + new InvalidOperationException("Progress reporter is creating a section with local progress!"), + "TGS BUG: A progress reporter is using mixed local and nested progress, this is not supported"); + } + + sectionReservations = 0; + } + + // floating point >.< + if (percentage + sectionReservations.Value > 1.0001) + { + var remainingPercentage = 1.0 - sectionReservations.Value; logger.LogError( "Stage {newStageName} is overbudgeted ({budget}/{remainingPercentage})! Clamping...", newStageName, @@ -121,6 +209,9 @@ namespace Tgstation.Server.Host.Jobs percentage = remainingPercentage; } + Math.Min(sectionReservations.Value + percentage, 1); + + var childLocalProgress = 0.0; var newReporter = new JobProgressReporter( logger, newStageName, @@ -133,11 +224,17 @@ namespace Tgstation.Server.Host.Jobs return; } - var childLocalProgress = progress.Value * percentage; + var progressWithoutChild = sectionProgression - childLocalProgress; + childLocalProgress = progress.Value * percentage; + + // floating point >.< + sectionProgression = Math.Min(progressWithoutChild + childLocalProgress, 1); + if (sectionProgression > 9.9999) + sectionProgression = 1; - sectionProgression = childLocalProgress + childBaseProgress; callback(currentStage, sectionProgression); - }); + }, + false); newReporter.ReportProgress(0); return newReporter; diff --git a/src/Tgstation.Server.Host/Jobs/JobService.cs b/src/Tgstation.Server.Host/Jobs/JobService.cs index e9967f5189..d78cc7a287 100644 --- a/src/Tgstation.Server.Host/Jobs/JobService.cs +++ b/src/Tgstation.Server.Host/Jobs/JobService.cs @@ -460,14 +460,16 @@ namespace Tgstation.Server.Host.Jobs QueueHubUpdate(job.ToApi(), false); logger.LogTrace("Starting job..."); + using var progressReporter = new JobProgressReporter( + loggerFactory.CreateLogger(), + null, + UpdateProgress); + using var innerReporter = progressReporter.CreateSection(null, 1.0); await operation( instanceCoreProvider.GetInstance(job.Instance!), databaseContextFactory, job, - new JobProgressReporter( - loggerFactory.CreateLogger(), - null, - UpdateProgress), + innerReporter, cancellationToken); logger.LogDebug("Job {jobId} completed!", job.Id); diff --git a/src/Tgstation.Server.Host/System/IProcessBase.cs b/src/Tgstation.Server.Host/System/IProcessBase.cs index f34d5e6597..6ae88dce50 100644 --- a/src/Tgstation.Server.Host/System/IProcessBase.cs +++ b/src/Tgstation.Server.Host/System/IProcessBase.cs @@ -16,7 +16,7 @@ namespace Tgstation.Server.Host.System /// /// Gets the process' memory usage in bytes. /// - long MemoryUsage { get; } + long? MemoryUsage { get; } /// /// Set's the owned to a non-normal value. diff --git a/src/Tgstation.Server.Host/System/Process.cs b/src/Tgstation.Server.Host/System/Process.cs index 64c92bf89b..13a1a278f5 100644 --- a/src/Tgstation.Server.Host/System/Process.cs +++ b/src/Tgstation.Server.Host/System/Process.cs @@ -23,7 +23,21 @@ namespace Tgstation.Server.Host.System public Task Lifetime { get; } /// - public long MemoryUsage => handle.VirtualMemorySize64; + public long? MemoryUsage + { + get + { + try + { + return handle.VirtualMemorySize64; + } + catch (Exception ex) + { + logger.LogWarning(ex, "Failed to get PID {pid}'s memory usage!", Id); + return null; + } + } + } /// /// The for the . diff --git a/tests/Tgstation.Server.Host.Tests/Components/Engine/TestOpenDreamInstaller.cs b/tests/Tgstation.Server.Host.Tests/Components/Engine/TestOpenDreamInstaller.cs index 2a62fea29a..8950acc6ab 100644 --- a/tests/Tgstation.Server.Host.Tests/Components/Engine/TestOpenDreamInstaller.cs +++ b/tests/Tgstation.Server.Host.Tests/Components/Engine/TestOpenDreamInstaller.cs @@ -13,6 +13,7 @@ using Tgstation.Server.Common.Http; using Tgstation.Server.Host.Components.Repository; using Tgstation.Server.Host.Configuration; using Tgstation.Server.Host.IO; +using Tgstation.Server.Host.Jobs; using Tgstation.Server.Host.System; using Tgstation.Server.Host.Utils; @@ -52,7 +53,7 @@ namespace Tgstation.Server.Host.Components.Engine.Tests null, null, null, - null, + It.IsNotNull(), true, It.IsAny())) .Callback(() => ++cloneAttempts) @@ -85,7 +86,7 @@ namespace Tgstation.Server.Host.Components.Engine.Tests Engine = EngineType.OpenDream, SourceSHA = new string('a', Limits.MaximumCommitShaLength), }, - null, + new JobProgressReporter(), CancellationToken.None); diff --git a/tests/Tgstation.Server.Host.Tests/Components/Repository/TestRepositoryManager.cs b/tests/Tgstation.Server.Host.Tests/Components/Repository/TestRepositoryManager.cs index c4a17f5140..2cdd59d19b 100644 --- a/tests/Tgstation.Server.Host.Tests/Components/Repository/TestRepositoryManager.cs +++ b/tests/Tgstation.Server.Host.Tests/Components/Repository/TestRepositoryManager.cs @@ -15,6 +15,7 @@ using Remora.Rest.Core; using Tgstation.Server.Host.Components.Events; using Tgstation.Server.Host.Configuration; using Tgstation.Server.Host.IO; +using Tgstation.Server.Host.Jobs; namespace Tgstation.Server.Host.Components.Repository.Tests { @@ -86,7 +87,7 @@ namespace Tgstation.Server.Host.Components.Repository.Tests null, null, null, - null, + new JobProgressReporter(), false, CancellationToken.None); diff --git a/tests/Tgstation.Server.Host.Tests/Jobs/TestJobProgressReporter.cs b/tests/Tgstation.Server.Host.Tests/Jobs/TestJobProgressReporter.cs new file mode 100644 index 0000000000..f6d6cd9348 --- /dev/null +++ b/tests/Tgstation.Server.Host.Tests/Jobs/TestJobProgressReporter.cs @@ -0,0 +1,66 @@ +using Microsoft.Extensions.Logging; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +using Moq; + +namespace Tgstation.Server.Host.Jobs.Tests +{ + [TestClass] + public sealed class TestJobProgressReporter + { + string expectedStageName = null; + double? expectedProgress = null; + void Validate(string stageName, double? progress) + { + Assert.AreEqual(expectedStageName, stageName); + Assert.AreEqual(expectedProgress, progress); + } + + JobProgressReporter Setup() + { + expectedStageName = null; + expectedProgress = 0; + return new JobProgressReporter( + Mock.Of>(), + null, + Validate); + } + + [TestMethod] + public void TestBasicUsage() + { + var progressReporter = Setup(); + + expectedProgress = 0.4; + progressReporter.ReportProgress(0.4); + expectedProgress = 1.0; + progressReporter.ReportProgress(1.0); + } + + [TestMethod] + public void TestNestedUsage() + { + var progressReporter = Setup(); + + expectedStageName = "Test1"; + var subReporter1 = progressReporter.CreateSection("Test1", 0.5); + expectedProgress = 0.1; + subReporter1.ReportProgress(0.2); + expectedProgress = 0.4; + subReporter1.ReportProgress(0.8); + + expectedStageName = "Test2"; + var subReporter2 = progressReporter.CreateSection("Test2", 0.5); + + expectedStageName = "Test1"; + expectedProgress = 0.5; + subReporter1.ReportProgress(1); + + expectedStageName = "Test2"; + expectedProgress = 0.6; + subReporter2.ReportProgress(0.2); + expectedProgress = 1.0; + subReporter2.ReportProgress(1); + } + } +} diff --git a/tests/Tgstation.Server.Tests/Live/Instance/InstanceTest.cs b/tests/Tgstation.Server.Tests/Live/Instance/InstanceTest.cs index 9c2b44c962..67ed9e2178 100644 --- a/tests/Tgstation.Server.Tests/Live/Instance/InstanceTest.cs +++ b/tests/Tgstation.Server.Tests/Live/Instance/InstanceTest.cs @@ -22,6 +22,7 @@ using Tgstation.Server.Host.Components.Events; using Tgstation.Server.Host.Components.Repository; using Tgstation.Server.Host.Configuration; using Tgstation.Server.Host.IO; +using Tgstation.Server.Host.Jobs; using Tgstation.Server.Host.System; using Tgstation.Server.Host.Utils; @@ -142,7 +143,7 @@ namespace Tgstation.Server.Tests.Live.Instance using var windowsByondInstaller = byondInstaller as WindowsByondInstaller; // get the bytes for stable - return await byondInstaller.DownloadVersion(compatVersion, null, cancellationToken); + return await byondInstaller.DownloadVersion(compatVersion, new JobProgressReporter(), cancellationToken); } public async Task RunCompatTests(