From f1f0a350d6cfd5f127ecffe688b94ff2f5bba55a Mon Sep 17 00:00:00 2001 From: Jordan Date: Fri, 16 Jun 2023 17:35:08 -0400 Subject: [PATCH] PosixWatchdog doesn't need hard links - Added ISymlinkFactory.SymlinkedDirectoriesAreFiles because they are files on Linux. - Removed crazy-ass hard link junk from PosixWatchdog. - Added symlink unit tests. --- .../Deployment/SwappableDmbProvider.cs | 8 +- .../Components/Watchdog/PosixWatchdog.cs | 81 +------------- .../Components/Watchdog/WindowsWatchdog.cs | 2 +- .../IO/ISymlinkFactory.cs | 6 + .../IO/PosixSymlinkFactory.cs | 3 + .../IO/WindowsSymlinkFactory.cs | 3 + .../System/TestSymlinkFactory.cs | 105 ++++++++++++++++++ 7 files changed, 127 insertions(+), 81 deletions(-) create mode 100644 tests/Tgstation.Server.Host.Tests/System/TestSymlinkFactory.cs diff --git a/src/Tgstation.Server.Host/Components/Deployment/SwappableDmbProvider.cs b/src/Tgstation.Server.Host/Components/Deployment/SwappableDmbProvider.cs index b406267d0f..e496453222 100644 --- a/src/Tgstation.Server.Host/Components/Deployment/SwappableDmbProvider.cs +++ b/src/Tgstation.Server.Host/Components/Deployment/SwappableDmbProvider.cs @@ -80,9 +80,11 @@ namespace Tgstation.Server.Host.Components.Deployment if (Interlocked.Exchange(ref swapped, 1) != 0) throw new InvalidOperationException("Already swapped!"); - // Note this comment from TGS3: - // These next two lines should be atomic but this is the best we can do - await ioManager.DeleteDirectory(LiveGameDirectory, cancellationToken); + if (symlinkFactory.SymlinkedDirectoriesAreDeletedAsFiles) + await ioManager.DeleteFile(LiveGameDirectory, cancellationToken); + else + await ioManager.DeleteDirectory(LiveGameDirectory, cancellationToken); + await symlinkFactory.CreateSymbolicLink( ioManager.ResolvePath(baseProvider.Directory), ioManager.ResolvePath(LiveGameDirectory), diff --git a/src/Tgstation.Server.Host/Components/Watchdog/PosixWatchdog.cs b/src/Tgstation.Server.Host/Components/Watchdog/PosixWatchdog.cs index 69c5bcfb31..044ef3f1ef 100644 --- a/src/Tgstation.Server.Host/Components/Watchdog/PosixWatchdog.cs +++ b/src/Tgstation.Server.Host/Components/Watchdog/PosixWatchdog.cs @@ -1,5 +1,4 @@ -using System; -using System.Threading; +using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.Logging; @@ -22,11 +21,6 @@ namespace Tgstation.Server.Host.Components.Watchdog /// sealed class PosixWatchdog : WindowsWatchdog { - /// - /// If the swappable game directory is currently a rename of the compile job. - /// - IDmbProvider hardLinkedDmb; - /// /// Initializes a new instance of the class. /// @@ -84,77 +78,10 @@ namespace Tgstation.Server.Host.Components.Watchdog } /// - protected override Task ApplyInitialDmb(CancellationToken cancellationToken) => Task.CompletedTask; // not necessary to hold initial .dmb on Linux because of based inode deletes - - /// - protected override async Task InitialLink(CancellationToken cancellationToken) + protected override Task ApplyInitialDmb(CancellationToken cancellationToken) { - // The logic to check for an active live directory is in SwappableDmbProvider, so we just do it again here for safety - Logger.LogTrace("Hard linking compile job..."); - - // Symlinks are counted as a file on linux?? - if (await GameIOManager.DirectoryExists(ActiveSwappable.Directory, cancellationToken)) - await GameIOManager.DeleteDirectory(ActiveSwappable.Directory, cancellationToken); - else - await GameIOManager.DeleteFile(ActiveSwappable.Directory, cancellationToken); - - // Instead of symlinking to begin with we actually rename the directory - await GameIOManager.MoveDirectory( - ActiveSwappable.CompileJob.DirectoryName.ToString(), - ActiveSwappable.Directory, - cancellationToken); - - hardLinkedDmb = ActiveSwappable; - } - - /// - protected override async Task InitController(Task chatTask, ReattachInformation reattachInfo, CancellationToken cancellationToken) - { - var suspended = false; - try - { - await base.InitController(chatTask, reattachInfo, cancellationToken); - } - finally - { - // Then we move it back and apply the symlink - if (hardLinkedDmb != null) - { - try - { - Logger.LogTrace("Unhardlinking compile job..."); - Server?.Suspend(); - suspended = true; - var hardLink = hardLinkedDmb.Directory; - var originalPosition = hardLinkedDmb.CompileJob.DirectoryName.ToString(); - await GameIOManager.MoveDirectory( - hardLink, - originalPosition, - default); - } - catch (Exception ex) - { - Logger.LogError( - ex, - "Failed to un-hard link compile job #{compileJobId} ({compileJobDirectory})", - hardLinkedDmb.CompileJob.Id, - hardLinkedDmb.CompileJob.DirectoryName); - } - - hardLinkedDmb = null; - } - } - - if (reattachInfo != null) - { - Logger.LogTrace("Skipping symlink due to reattach"); - return; - } - - Logger.LogTrace("Symlinking compile job..."); - await ActiveSwappable.MakeActive(cancellationToken); - if (suspended) - Server.Resume(); + // not necessary to hold initial .dmb on Linux because of based inode deletes + return Task.CompletedTask; } } } diff --git a/src/Tgstation.Server.Host/Components/Watchdog/WindowsWatchdog.cs b/src/Tgstation.Server.Host/Components/Watchdog/WindowsWatchdog.cs index 04175fde30..3a37cf69ec 100644 --- a/src/Tgstation.Server.Host/Components/Watchdog/WindowsWatchdog.cs +++ b/src/Tgstation.Server.Host/Components/Watchdog/WindowsWatchdog.cs @@ -318,7 +318,7 @@ namespace Tgstation.Server.Host.Components.Watchdog /// /// The for the operation. /// A representing the running operation. - protected virtual Task InitialLink(CancellationToken cancellationToken) + Task InitialLink(CancellationToken cancellationToken) { Logger.LogTrace("Symlinking compile job..."); return ActiveSwappable.MakeActive(cancellationToken); diff --git a/src/Tgstation.Server.Host/IO/ISymlinkFactory.cs b/src/Tgstation.Server.Host/IO/ISymlinkFactory.cs index c762395fca..1adbd99d97 100644 --- a/src/Tgstation.Server.Host/IO/ISymlinkFactory.cs +++ b/src/Tgstation.Server.Host/IO/ISymlinkFactory.cs @@ -8,6 +8,12 @@ namespace Tgstation.Server.Host.IO /// interface ISymlinkFactory { + /// + /// If directory symlinks must be deleted as files would in the current environment. + /// + /// This is because Linux symlinked directories must be deleted with . + bool SymlinkedDirectoriesAreDeletedAsFiles { get; } + /// /// Create a symbolic link. /// diff --git a/src/Tgstation.Server.Host/IO/PosixSymlinkFactory.cs b/src/Tgstation.Server.Host/IO/PosixSymlinkFactory.cs index 4644080a10..c3c1abcc32 100644 --- a/src/Tgstation.Server.Host/IO/PosixSymlinkFactory.cs +++ b/src/Tgstation.Server.Host/IO/PosixSymlinkFactory.cs @@ -12,6 +12,9 @@ namespace Tgstation.Server.Host.IO /// sealed class PosixSymlinkFactory : ISymlinkFactory { + /// + public bool SymlinkedDirectoriesAreDeletedAsFiles => true; + /// public Task CreateSymbolicLink(string targetPath, string linkPath, CancellationToken cancellationToken) => Task.Factory.StartNew( () => diff --git a/src/Tgstation.Server.Host/IO/WindowsSymlinkFactory.cs b/src/Tgstation.Server.Host/IO/WindowsSymlinkFactory.cs index 2beddb0581..b2ced648e0 100644 --- a/src/Tgstation.Server.Host/IO/WindowsSymlinkFactory.cs +++ b/src/Tgstation.Server.Host/IO/WindowsSymlinkFactory.cs @@ -13,6 +13,9 @@ namespace Tgstation.Server.Host.IO /// sealed class WindowsSymlinkFactory : ISymlinkFactory { + /// + public bool SymlinkedDirectoriesAreDeletedAsFiles => false; + /// public Task CreateSymbolicLink(string targetPath, string linkPath, CancellationToken cancellationToken) => Task.Factory.StartNew( () => diff --git a/tests/Tgstation.Server.Host.Tests/System/TestSymlinkFactory.cs b/tests/Tgstation.Server.Host.Tests/System/TestSymlinkFactory.cs new file mode 100644 index 0000000000..8e488f11b0 --- /dev/null +++ b/tests/Tgstation.Server.Host.Tests/System/TestSymlinkFactory.cs @@ -0,0 +1,105 @@ +using System; +using System.IO; +using System.Threading; +using System.Threading.Tasks; + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +using Tgstation.Server.Host.IO; + +namespace Tgstation.Server.Host.System.Tests +{ + [TestClass] + public sealed class TestSymlinkFactory + { + readonly ISymlinkFactory factory = new PlatformIdentifier().IsWindows + ? new WindowsSymlinkFactory() + : new PosixSymlinkFactory(); + + [TestMethod] + public async Task TestSymlinks() + { + var cancellationToken = CancellationToken.None; + var cwd = Path.GetTempFileName(); + File.Delete(cwd); + Directory.CreateDirectory(cwd); + try + { + var realDir = Path.Combine(cwd, "RealDir"); + var symDir = Path.Combine(cwd, "SymDir"); + var realFile = Path.Combine(cwd, "RealFile.txt"); + var symFile = Path.Combine(realDir, "RealFile.txt"); + + var subRealFile = Path.Combine(realDir, "test.txt"); + var subSymFile = Path.Combine(symDir, "test.txt"); + + try + { + await factory.CreateSymbolicLink(subRealFile, subSymFile, cancellationToken); + Assert.Fail("Expected Exception!"); + } + catch + { + } + + Directory.CreateDirectory(realDir); + Directory.CreateDirectory(symDir); + + await File.WriteAllBytesAsync(realFile, Array.Empty(), cancellationToken); + await File.WriteAllBytesAsync(symFile, Array.Empty(), cancellationToken); + + try + { + await factory.CreateSymbolicLink(realFile, symFile, cancellationToken); + Assert.Fail("Expected Exception!"); + } + catch + { + } + + Directory.Delete(symDir); + File.Delete(symFile); + + await factory.CreateSymbolicLink(realFile, symFile, cancellationToken); + Assert.IsTrue(File.Exists(symFile)); + Assert.IsFalse(Directory.Exists(symFile)); + + await File.WriteAllTextAsync(realFile, "test", cancellationToken); + var symFileContents = await File.ReadAllTextAsync(symFile, cancellationToken); + Assert.AreEqual("test", symFileContents); + File.Delete(symFile); + File.Delete(realFile); + + try + { + await factory.CreateSymbolicLink(realDir, symDir, cancellationToken); + + Assert.IsFalse(File.Exists(symDir)); + Assert.IsTrue(Directory.Exists(symDir)); + + await File.WriteAllTextAsync(subRealFile, "test", cancellationToken); + Assert.IsTrue(File.Exists(subSymFile)); + + File.Delete(subSymFile); + Assert.IsFalse(File.Exists(subRealFile)); + } + finally + { + if (factory.SymlinkedDirectoriesAreDeletedAsFiles) + File.Delete(symDir); + else + Directory.Delete(symDir); + } + + if (factory.SymlinkedDirectoriesAreDeletedAsFiles) + Assert.IsFalse(File.Exists(symDir)); + else + Assert.IsFalse(Directory.Exists(symDir)); + } + finally + { + Directory.Delete(cwd, true); + } + } + } +}