Let SQL Server and PostgreSQL primaries be upgraded

src/ServiceControl.Config.Tests/UpgradeMaintenancePortTests.cs

@@ -0,0 +1,55 @@
+namespace ServiceControl.Config.Tests
+{
+    using System.Collections.Generic;
+    using System.Linq;
+    using Commands;
+    using NUnit.Framework;
+    using ServiceControlInstaller.Engine.Configuration.ServiceControl;
+    using ServiceControlInstaller.Engine.Instances;
+
+    [TestFixture]
+    public class UpgradeMaintenancePortTests
+    {
+        static readonly Dictionary<string, bool> PrimaryUsesMaintenancePort = new()
+        {
+            ["RavenDB"] = true,
+            ["RavenDB35"] = true,
+            ["SQLServer"] = false,
+            ["PostgreSQL"] = false
+        };
+
+        [Test]
+        public void Every_shipped_primary_persister_gets_its_decided_maintenance_port_answer()
+        {
+            var manifests = ServiceControlPersisters.GetAllPrimaryManifests();
+
+            Assert.That(manifests.Select(m => m.Name).Distinct(), Is.EquivalentTo(PrimaryUsesMaintenancePort.Keys),
+                "The shipped persisters changed. Decide whether each new one needs the maintenance port prompt and add it to the table.");
+
+            Assert.Multiple(() =>
+            {
+                foreach (var manifest in manifests)
+                {
+                    Assert.That(UpgradeServiceControlInstanceCommand.UsesMaintenancePort(manifest), Is.EqualTo(PrimaryUsesMaintenancePort[manifest.Name]), manifest.Name);
+                }
+            });
+        }
+
+        [Test]
+        public void The_answer_follows_the_declared_setting_not_the_persister_name()
+        {
+            var ravenWithoutPort = new PersistenceManifest { Name = "RavenDB", Settings = [] };
+            var otherWithPort = new PersistenceManifest
+            {
+                Name = "SomeFuturePersister",
+                Settings = [new PersistenceManifest.Setting { Name = ServiceControlSettings.DatabaseMaintenancePort.Name }]
+            };
+
+            Assert.Multiple(() =>
+            {
+                Assert.That(UpgradeServiceControlInstanceCommand.UsesMaintenancePort(ravenWithoutPort), Is.False);
+                Assert.That(UpgradeServiceControlInstanceCommand.UsesMaintenancePort(otherWithPort), Is.True);
+            });
+        }
+    }
+}

src/ServiceControl.Config/Commands/UpgradeServiceControlInstanceCommand.cs

@@ -1,6 +1,7 @@
 namespace ServiceControl.Config.Commands
 {
     using System;
+    using System.Linq;
     using System.Threading;
     using System.Threading.Tasks;
     using Caliburn.Micro;
@@ -100,7 +101,7 @@ public override async Task ExecuteAsync(InstanceDetailsViewModel model)
                 }
             }
 
-            if (!instance.AppConfig.AppSettingExists(ServiceControlSettings.DatabaseMaintenancePort.Name))
+            if (UsesMaintenancePort(instance.PersistenceManifest) && !instance.AppConfig.AppSettingExists(ServiceControlSettings.DatabaseMaintenancePort.Name))
             {
                 var viewModel = new TextBoxDialogViewModel("INPUT REQUIRED - MAINTENANCE PORT",
                     "When Service Control is set to maintenance mode it requires a prereserved port on which it exposes the RavenDB database.",
@@ -193,6 +194,9 @@ async Task UpgradeServiceControlInstance(InstanceDetailsViewModel model, Service
             }
         }
 
+        internal static bool UsesMaintenancePort(PersistenceManifest manifest) =>
+            manifest.Settings.Any(setting => setting.Name == ServiceControlSettings.DatabaseMaintenancePort.Name);
+
         readonly IEventAggregator eventAggregator;
         readonly IServiceControlWindowManager windowManager;
         readonly ServiceControlInstanceInstaller serviceControlInstaller;

src/ServiceControlInstaller.Engine.UnitTests/Services/TryStopServiceTests.cs

@@ -0,0 +1,57 @@
+namespace ServiceControlInstaller.Engine.UnitTests.Services
+{
+    using System;
+    using System.IO;
+    using System.ServiceProcess;
+    using System.Threading.Tasks;
+    using Engine.Services;
+    using Instances;
+    using NUnit.Framework;
+
+    [TestFixture]
+    public class TryStopServiceTests
+    {
+        string installPath;
+
+        [SetUp]
+        public void SetUp()
+        {
+            installPath = Path.Combine(Path.GetTempPath(), "TryStopServiceTests", Guid.NewGuid().ToString("N"));
+            Directory.CreateDirectory(installPath);
+        }
+
+        [TearDown]
+        public void TearDown() => Directory.Delete(installPath, true);
+
+        [Test]
+        public async Task Waits_for_files_released_more_than_five_seconds_after_the_service_stops()
+        {
+            var lockedDll = new FileStream(Path.Combine(installPath, "Locked.dll"), FileMode.Create, FileAccess.ReadWrite, FileShare.None);
+            var release = Task.Delay(TimeSpan.FromSeconds(7)).ContinueWith(_ => lockedDll.Dispose());
+
+            var instance = new ServiceControlInstance(new StoppedServiceController(Path.Combine(installPath, "ServiceControl.exe")));
+
+            Assert.That(instance.TryStopService(), Is.True);
+
+            await release;
+        }
+
+        class StoppedServiceController(string exePath) : IWindowsServiceController
+        {
+            public string ServiceName => "ServiceControl";
+            public string ExePath { get; } = exePath;
+            public ServiceControllerStatus Status => ServiceControllerStatus.Running;
+            public string Account => "system";
+            public string DisplayName => throw new NotImplementedException();
+            public string Description { get => throw new NotImplementedException(); set => throw new NotImplementedException(); }
+            public bool Exists() => true;
+            public void Refresh() { }
+            public void Stop() { }
+            public void WaitForStatus(ServiceControllerStatus stopped, TimeSpan timeSpan) { }
+            public void Start() => throw new NotImplementedException();
+            public void Delete() => throw new NotImplementedException();
+            public void SetStartupMode(string v) => throw new NotImplementedException();
+            public void ChangeAccountDetails(string accountName, string serviceAccountPwd) => throw new NotImplementedException();
+        }
+    }
+}

src/ServiceControlInstaller.Engine.UnitTests/Validation/StorageEngineUpgradeTests.cs

@@ -0,0 +1,70 @@
+namespace ServiceControlInstaller.Engine.UnitTests.Validation
+{
+    using System.Collections.Generic;
+    using System.Linq;
+    using Instances;
+    using NUnit.Framework;
+
+    [TestFixture]
+    public class StorageEngineUpgradeTests
+    {
+        static readonly Dictionary<string, bool> PrimaryUpgradableInPlace = new()
+        {
+            ["RavenDB"] = true,
+            ["RavenDB35"] = false,
+            ["SQLServer"] = true,
+            ["PostgreSQL"] = true
+        };
+
+        static readonly Dictionary<string, bool> AuditUpgradableInPlace = new()
+        {
+            ["RavenDB"] = true,
+            ["RavenDB35"] = false
+        };
+
+        [Test]
+        public void Every_shipped_primary_storage_engine_gets_its_decided_upgrade_answer() =>
+            AssertDecidedAnswers(ServiceControlPersisters.GetAllPrimaryManifests(), PrimaryUpgradableInPlace);
+
+        [Test]
+        public void Every_shipped_audit_storage_engine_gets_its_decided_upgrade_answer() =>
+            AssertDecidedAnswers(ServiceControlPersisters.GetAllAuditManifests(), AuditUpgradableInPlace);
+
+        // A null persistence type is how a version 4 config with no PersistenceType setting resolves, which is RavenDB 3.5.
+        [TestCase(null, ExpectedResult = false)]
+        [TestCase("NotAKnownPersister", ExpectedResult = false)]
+        public bool Whether_an_unlisted_primary_persistence_type_can_be_upgraded_in_place(string persistenceType) =>
+            AbstractCommandChecks.IsUpgradableStorageEngine(ServiceControlPersisters.GetPrimaryPersistence(persistenceType));
+
+        [TestCase(null, ExpectedResult = false)]
+        [TestCase("NotAKnownPersister", ExpectedResult = false)]
+        [TestCase("ServiceControl.Audit.Persistence.RavenDb.RavenDbPersistenceConfiguration, ServiceControl.Audit.Persistence.RavenDb5", ExpectedResult = true)]
+        [TestCase("ServiceControl.Audit.Persistence.RavenDb.RavenDbPersistenceConfiguration, ServiceControl.Audit.Persistence.RavenDb", ExpectedResult = false)]
+        public bool Whether_an_unlisted_audit_persistence_type_can_be_upgraded_in_place(string persistenceType) =>
+            AbstractCommandChecks.IsUpgradableStorageEngine(ServiceControlPersisters.GetAuditPersistence(persistenceType));
+
+        [Test]
+        public void The_answer_follows_whether_the_manifest_is_supported_not_its_name()
+        {
+            Assert.Multiple(() =>
+            {
+                Assert.That(AbstractCommandChecks.IsUpgradableStorageEngine(new PersistenceManifest { Name = "SomeFuturePersister", IsSupported = true }), Is.True);
+                Assert.That(AbstractCommandChecks.IsUpgradableStorageEngine(new PersistenceManifest { Name = "RavenDB", IsSupported = false }), Is.False);
+            });
+        }
+
+        static void AssertDecidedAnswers(PersistenceManifest[] manifests, Dictionary<string, bool> decided)
+        {
+            Assert.That(manifests.Select(m => m.Name).Distinct(), Is.EquivalentTo(decided.Keys),
+                "The shipped persisters changed. Decide whether each new one can be upgraded in place and add it to the table.");
+
+            Assert.Multiple(() =>
+            {
+                foreach (var manifest in manifests)
+                {
+                    Assert.That(AbstractCommandChecks.IsUpgradableStorageEngine(manifest), Is.EqualTo(decided[manifest.Name]), manifest.Name);
+                }
+            });
+        }
+    }
+}

src/ServiceControlInstaller.Engine/Instances/BaseService.cs

@@ -87,7 +87,7 @@ public bool TryStopService()
                     }
                 });
 
-                return t.Wait(TimeSpan.FromSeconds(5));
+                return t.Wait(TimeSpan.FromSeconds(60));
             }
             catch (TimeoutException)
             {

src/ServiceControlInstaller.Engine/Validation/AbstractCommandChecks.cs

@@ -146,9 +146,7 @@ public async Task<bool> CanUpgradeInstance(BaseService instance, bool forceUpgra
                 }
                 else
                 {
-                    var compatibleStorageEngine = baseInstance.PersistenceManifest.Name == StorageEngineNames.RavenDB;
-
-                    if (!compatibleStorageEngine)
+                    if (!IsUpgradableStorageEngine(baseInstance.PersistenceManifest))
                     {
                         await NotifyForIncompatibleStorageEngine(baseInstance, cancellationToken).ConfigureAwait(false);
                         return false;
@@ -173,6 +171,9 @@ public async Task<bool> CanUpgradeInstance(BaseService instance, bool forceUpgra
             return true;
         }
 
+        // RavenDB 3.5 and unknown persisters are the ones marked unsupported, and neither can be upgraded in place.
+        internal static bool IsUpgradableStorageEngine(PersistenceManifest manifest) => manifest.IsSupported;
+
         async Task<bool> OldVersionOfServiceControlInstalled(CancellationToken cancellationToken)
         {
             if (OldScmuCheck.OldVersionOfServiceControlInstalled(out var installedVersion))