Adds better port allocation

build/Version.props

@@ -4,8 +4,8 @@
     <!-- Integration tests will ensure they match across the board -->
     <TgsCoreVersion>4.5.4</TgsCoreVersion>
     <TgsConfigVersion>2.1.0</TgsConfigVersion>
-    <TgsApiVersion>7.3.2</TgsApiVersion>
-    <TgsClientVersion>8.3.2</TgsClientVersion>
+    <TgsApiVersion>7.4.0</TgsApiVersion>
+    <TgsClientVersion>8.4.0</TgsClientVersion>
     <TgsDmapiVersion>5.2.6</TgsDmapiVersion>
     <TgsControlPanelVersion>0.4.0</TgsControlPanelVersion>
     <TgsHostWatchdogVersion>1.1.0</TgsHostWatchdogVersion>

src/Tgstation.Server.Api/Models/ErrorCode.cs

@@ -570,5 +570,17 @@ namespace Tgstation.Server.Api.Models
 		/// </summary>
 		[Description("Failed to allow DreamDaemon through the Windows firewall!")]
 		ByondDreamDaemonFirewallFail,
+
+		/// <summary>
+		/// Attempted to create an instance but no free ports could be found.
+		/// </summary>
+		[Description("TGS was unable to find a free port to allocate for the operation!")]
+		NoPortsAvailable,
+
+		/// <summary>
+		/// Attempted to set a port which is either in use by another part of TGS or otherwise not available for binding.
+		/// </summary>
+		[Description("The requested port is either already in use by TGS or could not be allocated!")]
+		PortNotAvailable,
 	}
 }

src/Tgstation.Server.Host/Controllers/DreamDaemonController.cs

@@ -12,6 +12,7 @@ using Tgstation.Server.Api.Models;
 using Tgstation.Server.Api.Rights;
 using Tgstation.Server.Host.Components;
 using Tgstation.Server.Host.Components.Session;
+using Tgstation.Server.Host.Core;
 using Tgstation.Server.Host.Database;
 using Tgstation.Server.Host.Jobs;
 using Tgstation.Server.Host.Models;
@@ -26,10 +27,15 @@ namespace Tgstation.Server.Host.Controllers
 	public sealed class DreamDaemonController : InstanceRequiredController
 	{
 		/// <summary>
-		/// The <see cref="IJobManager"/> for the <see cref="DreamMakerController"/>
+		/// The <see cref="IJobManager"/> for the <see cref="DreamDaemonController"/>.
 		/// </summary>
 		readonly IJobManager jobManager;
 
+		/// <summary>
+		/// The <see cref="IPortAllocator"/> for the <see cref="DreamDaemonController"/>.
+		/// </summary>
+		readonly IPortAllocator portAllocator;
+
 		/// <summary>
 		/// Construct a <see cref="DreamDaemonController"/>
 		/// </summary>
@@ -37,12 +43,14 @@ namespace Tgstation.Server.Host.Controllers
 		/// <param name="authenticationContextFactory">The <see cref="IAuthenticationContextFactory"/> for the <see cref="ApiController"/></param>
 		/// <param name="jobManager">The value of <see cref="jobManager"/></param>
 		/// <param name="instanceManager">The <see cref="IInstanceManager"/> for the <see cref="InstanceRequiredController"/>.</param>
+		/// <param name="portAllocator">The value of <see cref="IPortAllocator"/>.</param>
 		/// <param name="logger">The <see cref="ILogger"/> for the <see cref="ApiController"/></param>
 		public DreamDaemonController(
 			IDatabaseContext databaseContext,
 			IAuthenticationContextFactory authenticationContextFactory,
 			IJobManager jobManager,
 			IInstanceManager instanceManager,
+			IPortAllocator portAllocator,
 			ILogger<DreamDaemonController> logger)
 			: base(
 				  instanceManager,
@@ -51,6 +59,7 @@ namespace Tgstation.Server.Host.Controllers
 				  logger)
 		{
 			this.jobManager = jobManager ?? throw new ArgumentNullException(nameof(jobManager));
+			this.portAllocator = portAllocator ?? throw new ArgumentNullException(nameof(portAllocator));
 		}
 
 		/// <summary>
@@ -219,6 +228,18 @@ namespace Tgstation.Server.Host.Controllers
 			if (current == default)
 				return Gone();
 
+			if (model.Port.HasValue && model.Port.Value != current.Port.Value)
+			{
+				var verifiedPort = await portAllocator
+					.GetAvailablePort(
+						model.Port.Value,
+						true,
+						cancellationToken)
+					.ConfigureAwait(false);
+				if (verifiedPort != model.Port)
+					return Conflict(new ErrorMessage(ErrorCode.PortNotAvailable));
+			}
+
 			var userRights = (DreamDaemonRights)AuthenticationContext.GetRight(RightsType.DreamDaemon);
 
 			bool CheckModified<T>(Expression<Func<Api.Models.Internal.DreamDaemonSettings, T>> expression, DreamDaemonRights requiredRight)

src/Tgstation.Server.Host/Controllers/DreamMakerController.cs

@@ -1,4 +1,4 @@
-using Microsoft.AspNetCore.Mvc;
+using Microsoft.AspNetCore.Mvc;
 using Microsoft.EntityFrameworkCore;
 using Microsoft.Extensions.Logging;
 using System;
@@ -10,6 +10,7 @@ using Tgstation.Server.Api;
 using Tgstation.Server.Api.Models;
 using Tgstation.Server.Api.Rights;
 using Tgstation.Server.Host.Components;
+using Tgstation.Server.Host.Core;
 using Tgstation.Server.Host.Database;
 using Tgstation.Server.Host.Jobs;
 using Tgstation.Server.Host.Models;
@@ -29,6 +30,11 @@ namespace Tgstation.Server.Host.Controllers
 		/// </summary>
 		readonly IJobManager jobManager;
 
+		/// <summary>
+		/// The <see cref="IPortAllocator"/> for the <see cref="DreamMakerController"/>.
+		/// </summary>
+		readonly IPortAllocator portAllocator;
+
 		/// <summary>
 		/// Construct a <see cref="DreamMakerController"/>
 		/// </summary>
@@ -36,12 +42,14 @@ namespace Tgstation.Server.Host.Controllers
 		/// <param name="authenticationContextFactory">The <see cref="IAuthenticationContextFactory"/> for the <see cref="ApiController"/></param>
 		/// <param name="jobManager">The value of <see cref="jobManager"/></param>
 		/// <param name="instanceManager">The <see cref="IInstanceManager"/> for the <see cref="InstanceRequiredController"/>.</param>
+		/// <param name="portAllocator">The value of <see cref="IPortAllocator"/>.</param>
 		/// <param name="logger">The <see cref="ILogger"/> for the <see cref="ApiController"/></param>
 		public DreamMakerController(
 			IDatabaseContext databaseContext,
 			IAuthenticationContextFactory authenticationContextFactory,
 			IJobManager jobManager,
 			IInstanceManager instanceManager,
+			IPortAllocator portAllocator,
 			ILogger<DreamMakerController> logger)
 			: base(
 				  instanceManager,
@@ -50,6 +58,7 @@ namespace Tgstation.Server.Host.Controllers
 				  logger)
 		{
 			this.jobManager = jobManager ?? throw new ArgumentNullException(nameof(jobManager));
+			this.portAllocator = portAllocator ?? throw new ArgumentNullException(nameof(portAllocator));
 		}
 
 		/// <summary>
@@ -202,7 +211,20 @@ namespace Tgstation.Server.Host.Controllers
 			{
 				if (!AuthenticationContext.InstanceUser.DreamMakerRights.Value.HasFlag(DreamMakerRights.SetApiValidationPort))
 					return Forbid();
-				hostModel.ApiValidationPort = model.ApiValidationPort;
+
+				if (model.ApiValidationPort.Value != hostModel.ApiValidationPort.Value)
+				{
+					var verifiedPort = await portAllocator
+						.GetAvailablePort(
+							model.ApiValidationPort.Value,
+							true,
+							cancellationToken)
+							.ConfigureAwait(false);
+					if (verifiedPort != model.ApiValidationPort)
+						return Conflict(new ErrorMessage(ErrorCode.PortNotAvailable));
+
+					hostModel.ApiValidationPort = model.ApiValidationPort;
+				}
 			}
 
 			if (model.ApiValidationSecurityLevel.HasValue)

src/Tgstation.Server.Host/Controllers/InstanceController.cs

@@ -15,6 +15,7 @@ using Tgstation.Server.Api.Models;
 using Tgstation.Server.Api.Rights;
 using Tgstation.Server.Host.Components;
 using Tgstation.Server.Host.Configuration;
+using Tgstation.Server.Host.Core;
 using Tgstation.Server.Host.Database;
 using Tgstation.Server.Host.IO;
 using Tgstation.Server.Host.Jobs;
@@ -61,6 +62,11 @@ namespace Tgstation.Server.Host.Controllers
 		/// </summary>
 		readonly IPlatformIdentifier platformIdentifier;
 
+		/// <summary>
+		/// The <see cref="IPortAllocator"/> for the <see cref="InstanceController"/>.
+		/// </summary>
+		readonly IPortAllocator portAllocator;
+
 		/// <summary>
 		/// The <see cref="GeneralConfiguration"/> for the <see cref="InstanceController"/>.
 		/// </summary>
@@ -75,14 +81,16 @@ namespace Tgstation.Server.Host.Controllers
 		/// <param name="instanceManager">The value of <see cref="instanceManager"/></param>
 		/// <param name="ioManager">The value of <see cref="ioManager"/></param>
 		/// <param name="platformIdentifier">The value of <see cref="platformIdentifier"/></param>
+		/// <param name="portAllocator">The value of <see cref="IPortAllocator"/>.</param>
 		/// <param name="generalConfigurationOptions">The <see cref="IOptions{TOptions}"/> containing the value of <see cref="generalConfiguration"/>.</param>
-		/// <param name="logger">The <see cref="ILogger"/> for the <see cref="ApiController"/></param>
+		/// <param name="logger">The <see cref="ILogger"/> for the <see cref="ApiController"/>.</param>
 		public InstanceController(
 			IDatabaseContext databaseContext,
 			IAuthenticationContextFactory authenticationContextFactory,
 			IJobManager jobManager,
 			IInstanceManager instanceManager,
 			IIOManager ioManager,
+			IPortAllocator portAllocator,
 			IPlatformIdentifier platformIdentifier,
 			IOptions<GeneralConfiguration> generalConfigurationOptions,
 			ILogger<InstanceController> logger)
@@ -96,18 +104,43 @@ namespace Tgstation.Server.Host.Controllers
 			this.instanceManager = instanceManager ?? throw new ArgumentNullException(nameof(instanceManager));
 			this.ioManager = ioManager ?? throw new ArgumentNullException(nameof(ioManager));
 			this.platformIdentifier = platformIdentifier ?? throw new ArgumentNullException(nameof(platformIdentifier));
+			this.portAllocator = portAllocator ?? throw new ArgumentNullException(nameof(portAllocator));
 			generalConfiguration = generalConfigurationOptions?.Value ?? throw new ArgumentNullException(nameof(generalConfigurationOptions));
 		}
 
-		Models.Instance CreateDefaultInstance(Api.Models.Instance initialSettings)
-			=> new Models.Instance
+		async Task<Models.Instance> CreateDefaultInstance(Api.Models.Instance initialSettings, CancellationToken cancellationToken)
+		{
+			var ddPort = await portAllocator.GetAvailablePort(1, false, cancellationToken).ConfigureAwait(false);
+			if (!ddPort.HasValue)
+				return null;
+
+			// try to use the old default if possible
+			const ushort DefaultDreamDaemonPort = 1337;
+			if (ddPort.Value < DefaultDreamDaemonPort)
+				ddPort = await portAllocator.GetAvailablePort(DefaultDreamDaemonPort, false, cancellationToken).ConfigureAwait(false) ?? ddPort;
+
+			const ushort DefaultApiValidationPort = 1339;
+			var dmPort = await portAllocator
+				.GetAvailablePort(
+					Math.Min((ushort)(ddPort.Value + 1), DefaultApiValidationPort),
+					false,
+					cancellationToken)
+				.ConfigureAwait(false);
+			if (!dmPort.HasValue)
+				return null;
+
+			// try to use the old default if possible
+			if (dmPort < DefaultApiValidationPort)
+				dmPort = await portAllocator.GetAvailablePort(DefaultApiValidationPort, false, cancellationToken).ConfigureAwait(false) ?? dmPort;
+
+			return new Models.Instance
 			{
 				ConfigurationType = initialSettings.ConfigurationType ?? ConfigurationType.Disallowed,
 				DreamDaemonSettings = new DreamDaemonSettings
 				{
 					AllowWebClient = false,
 					AutoStart = false,
-					Port = 1337,
+					Port = ddPort,
 					SecurityLevel = DreamDaemonSecurity.Safe,
 					StartupTimeout = 60,
 					HeartbeatSeconds = 60,
@@ -115,7 +148,7 @@ namespace Tgstation.Server.Host.Controllers
 				},
 				DreamMakerSettings = new DreamMakerSettings
 				{
-					ApiValidationPort = 1339,
+					ApiValidationPort = dmPort,
 					ApiValidationSecurityLevel = DreamDaemonSecurity.Safe,
 					RequireDMApiValidation = true
 				},
@@ -140,6 +173,7 @@ namespace Tgstation.Server.Host.Controllers
 					InstanceAdminUser(null)
 				}
 			};
+		}
 
 		string NormalizePath(string path)
 		{
@@ -277,7 +311,9 @@ namespace Tgstation.Server.Host.Controllers
 				else
 					attached = true;
 
-			var newInstance = CreateDefaultInstance(model);
+			var newInstance = await CreateDefaultInstance(model, cancellationToken).ConfigureAwait(false);
+			if (newInstance == null)
+				return Conflict(new ErrorMessage(ErrorCode.NoPortsAvailable));
 
 			DatabaseContext.Instances.Add(newInstance);
 			try

src/Tgstation.Server.Host/Core/Application.cs

@@ -294,6 +294,7 @@ namespace Tgstation.Server.Host.Core
 			services.AddSingleton<IProcessExecutor, ProcessExecutor>();
 			services.AddSingleton<IServerPortProvider, ServerPortProivder>();
 			services.AddSingleton<ITopicClientFactory, TopicClientFactory>();
+			services.AddScoped<IPortAllocator, PortAllocator>();
 
 			// configure component services
 			services.AddSingleton<ILibGit2RepositoryFactory, LibGit2RepositoryFactory>();

src/Tgstation.Server.Host/Core/IPortAllocator.cs

@@ -0,0 +1,20 @@
+using System.Threading;
+using System.Threading.Tasks;
+
+namespace Tgstation.Server.Host.Core
+{
+	/// <summary>
+	/// Gets unassigned ports for use by TGS.
+	/// </summary>
+	public interface IPortAllocator
+	{
+		/// <summary>
+		/// Gets a port not currently in use by TGS.
+		/// </summary>
+		/// <param name="basePort">The port to check first. Will not allocate a port lower than this.</param>
+		/// <param name="checkOne">If only <paramref name="basePort"/> should be checked and no others.</param>
+		/// <param name="cancellationToken">The <see cref="CancellationToken"/> for the operation.</param>
+		/// <returns>A <see cref="Task{TResult}"/> resulting in the first available port on success, <see langword="null"/> on failure.</returns>
+		Task<ushort?> GetAvailablePort(ushort basePort, bool checkOne, CancellationToken cancellationToken);
+	}
+}

src/Tgstation.Server.Host/Core/PortAllocator.cs

@@ -0,0 +1,90 @@
+using Microsoft.EntityFrameworkCore;
+using Microsoft.Extensions.Logging;
+using System;
+using System.Linq;
+using System.Threading;
+using System.Threading.Tasks;
+using Tgstation.Server.Host.Database;
+using Tgstation.Server.Host.Extensions;
+
+namespace Tgstation.Server.Host.Core
+{
+	/// <inheritdoc />
+	sealed class PortAllocator : IPortAllocator
+	{
+		/// <summary>
+		/// The <see cref="IServerPortProvider"/> for the <see cref="PortAllocator"/>.
+		/// </summary>
+		readonly IServerPortProvider serverPortProvider;
+
+		/// <summary>
+		/// The <see cref="IDatabaseContext"/> for the <see cref="PortAllocator"/>.
+		/// </summary>
+		readonly IDatabaseContext databaseContext;
+
+		/// <summary>
+		/// The <see cref="ILogger"/> for the <see cref="PortAllocator"/>.
+		/// </summary>
+		readonly ILogger<PortAllocator> logger;
+
+		/// <summary>
+		/// Initializes a new instance of the <see cref="PortAllocator"/> <see langword="class"/>.
+		/// </summary>
+		/// <param name="serverPortProvider">The value of <see cref="serverPortProvider"/>.</param>
+		/// <param name="databaseContext">The value of <see cref="databaseContext"/>.</param>
+		/// <param name="logger">The value of <see cref="logger"/>.</param>
+		public PortAllocator(IServerPortProvider serverPortProvider, IDatabaseContext databaseContext, ILogger<PortAllocator> logger)
+		{
+			this.serverPortProvider = serverPortProvider ?? throw new ArgumentNullException(nameof(serverPortProvider));
+			this.databaseContext = databaseContext ?? throw new ArgumentNullException(nameof(databaseContext));
+			this.logger = logger ?? throw new ArgumentNullException(nameof(logger));
+		}
+
+		/// <inheritdoc />
+		public async Task<ushort?> GetAvailablePort(ushort basePort, bool checkOne, CancellationToken cancellationToken)
+		{
+			logger.LogTrace("Port allocation >= {0} requested...", basePort);
+
+			var ddPorts = await databaseContext
+				.DreamDaemonSettings
+				.AsQueryable()
+				.Select(x => x.Port)
+				.ToListAsync(cancellationToken)
+				.ConfigureAwait(false);
+
+			var dmPorts = await databaseContext
+				.DreamMakerSettings
+				.AsQueryable()
+				.Select(x => x.ApiValidationPort)
+				.ToListAsync(cancellationToken)
+				.ConfigureAwait(false);
+
+			for (var I = basePort; I < UInt16.MaxValue; ++I)
+			{
+				if (checkOne && I != basePort)
+					break;
+
+				if (I == serverPortProvider.HttpApiPort
+					|| ddPorts.Contains(I)
+					|| dmPorts.Contains(I))
+					continue;
+
+				try
+				{
+					logger.LogTrace("Bind test: {0}", I);
+					SocketExtensions.BindTest(I, false);
+				}
+				catch (Exception ex)
+				{
+					logger.LogDebug(ex, "Not using port {0}", I);
+				}
+
+				logger.LogInformation("Allocated port {0}", I);
+				return I;
+			}
+
+			logger.LogWarning("Unable to allocate port >= {0}!", basePort);
+			return null;
+		}
+	}
+}

tests/Tgstation.Server.Tests/Instance/DeploymentTest.cs

@@ -3,6 +3,7 @@ using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 using Tgstation.Server.Api.Models;
+using Tgstation.Server.Client;
 using Tgstation.Server.Client.Components;
 using Tgstation.Server.Host.System;
 
@@ -34,17 +35,42 @@ namespace Tgstation.Server.Tests.Instance
 
 			// by alphabetization rules, it should discover api_free here
 			if (!new PlatformIdentifier().IsWindows)
-				await dreamMakerClient.Update(new DreamMaker
+			{
+				var updatedDM = await dreamMakerClient.Update(new DreamMaker
 				{
 					ProjectName = "tests/DMAPI/ApiFree/api_free",
 					ApiValidationPort = IntegrationTest.DMPort
 				}, cancellationToken);
+				Assert.AreEqual(IntegrationTest.DMPort, updatedDM.ApiValidationPort);
+				Assert.AreEqual("tests/DMAPI/ApiFree/api_free", updatedDM.ProjectName);
+			}
+			else
+			{
+				var updatedDM = await dreamMakerClient.Update(new DreamMaker
+				{
+					ApiValidationPort = IntegrationTest.DMPort
+				}, cancellationToken);
+				Assert.AreEqual(IntegrationTest.DMPort, updatedDM.ApiValidationPort);
+			}
 
 			var updatedDD = await dreamDaemonClient.Update(new DreamDaemon
 			{
-				StartupTimeout = 5
+				StartupTimeout = 5,
+				Port = IntegrationTest.DDPort
 			}, cancellationToken);
 			Assert.AreEqual(5U, updatedDD.StartupTimeout);
+			Assert.AreEqual(IntegrationTest.DDPort, updatedDD.Port);
+
+			await ApiAssert.ThrowsException<ConflictException>(() => dreamDaemonClient.Update(new DreamDaemon
+			{
+				Port = IntegrationTest.DMPort
+			}, cancellationToken), ErrorCode.PortNotAvailable);
+
+			await ApiAssert.ThrowsException<ConflictException>(() => dreamMakerClient.Update(new DreamMaker
+			{
+				ApiValidationPort = IntegrationTest.DDPort
+			}, cancellationToken), ErrorCode.PortNotAvailable);
+
 			deployJob = await dreamMakerClient.Compile(cancellationToken);
 			await WaitForJob(deployJob, 30, true, ErrorCode.DreamMakerNeverValidated, cancellationToken);