Skip to content

Commit 337b6c0

Browse files
andreasohlundandreasohlund
authored
Do not kill instance setup to minimize the risk for a corrupted database (#4852)
* Wait indefinite for the setup to complete to minmize the risk for a corrupted database * Add setup process fake * Fix test * Check exceptions * Add test for non zero return codes * Add test to make sure do not kill the setup * Make sure we capture stderr * Suppress * Explain why we wait for ever
1 parent d0b1aee commit 337b6c0

File tree

6 files changed

+104
-17
lines changed

6 files changed

+104
-17
lines changed

‎src/ServiceControl.sln‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "ServiceControl.Transports.P
183183
EndProject
184184
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "ServiceControl.Transports.PostgreSql.Tests", "ServiceControl.Transports.PostgreSql.Tests\ServiceControl.Transports.PostgreSql.Tests.csproj", "{18DBEEF5-42EE-4C1D-A05B-87B21C067D53}"
185185
EndProject
186+
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "SetupProcessFake", "SetupProcessFake\SetupProcessFake.csproj", "{36D53BA0-C1E1-4D74-81AE-C33B40C84958}"
187+
EndProject
186188
Global
187189
GlobalSection(SolutionConfigurationPlatforms) = preSolution
188190
Debug|Any CPU = Debug|Any CPU
@@ -997,6 +999,18 @@ Global
997999
{18DBEEF5-42EE-4C1D-A05B-87B21C067D53}.Release|x64.Build.0 = Release|Any CPU
9981000
{18DBEEF5-42EE-4C1D-A05B-87B21C067D53}.Release|x86.ActiveCfg = Release|Any CPU
9991001
{18DBEEF5-42EE-4C1D-A05B-87B21C067D53}.Release|x86.Build.0 = Release|Any CPU
1002+
{36D53BA0-C1E1-4D74-81AE-C33B40C84958}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
1003+
{36D53BA0-C1E1-4D74-81AE-C33B40C84958}.Debug|Any CPU.Build.0 = Debug|Any CPU
1004+
{36D53BA0-C1E1-4D74-81AE-C33B40C84958}.Debug|x64.ActiveCfg = Debug|Any CPU
1005+
{36D53BA0-C1E1-4D74-81AE-C33B40C84958}.Debug|x64.Build.0 = Debug|Any CPU
1006+
{36D53BA0-C1E1-4D74-81AE-C33B40C84958}.Debug|x86.ActiveCfg = Debug|Any CPU
1007+
{36D53BA0-C1E1-4D74-81AE-C33B40C84958}.Debug|x86.Build.0 = Debug|Any CPU
1008+
{36D53BA0-C1E1-4D74-81AE-C33B40C84958}.Release|Any CPU.ActiveCfg = Release|Any CPU
1009+
{36D53BA0-C1E1-4D74-81AE-C33B40C84958}.Release|Any CPU.Build.0 = Release|Any CPU
1010+
{36D53BA0-C1E1-4D74-81AE-C33B40C84958}.Release|x64.ActiveCfg = Release|Any CPU
1011+
{36D53BA0-C1E1-4D74-81AE-C33B40C84958}.Release|x64.Build.0 = Release|Any CPU
1012+
{36D53BA0-C1E1-4D74-81AE-C33B40C84958}.Release|x86.ActiveCfg = Release|Any CPU
1013+
{36D53BA0-C1E1-4D74-81AE-C33B40C84958}.Release|x86.Build.0 = Release|Any CPU
10001014
EndGlobalSection
10011015
GlobalSection(SolutionProperties) = preSolution
10021016
HideSolutionNode = FALSE
@@ -1080,6 +1094,7 @@ Global
10801094
{51F5504E-E915-40EC-B96E-CA700A57982C} = {80C55E70-4B7A-4EF2-BB9E-C42F8DB0495D}
10811095
{448CBDCF-718D-4BC7-8F7C-099C9A362B59} = {A21A1A89-0B07-4E87-8E3C-41D9C280DCB8}
10821096
{18DBEEF5-42EE-4C1D-A05B-87B21C067D53} = {E0E45F22-35E3-4AD8-B09E-EFEA5A2F18EE}
1097+
{36D53BA0-C1E1-4D74-81AE-C33B40C84958} = {927A078A-E271-4878-A153-86D71AE510E2}
10831098
EndGlobalSection
10841099
GlobalSection(ExtensibilityGlobals) = postSolution
10851100
SolutionGuid = {3B9E5B72-F580-465A-A22C-2D2148AF4EB4}

‎src/ServiceControlInstaller.Engine.UnitTests/ServiceControlInstaller.Engine.UnitTests.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
<ItemGroup>
88
<ProjectReference Include="..\ServiceControlInstaller.Engine\ServiceControlInstaller.Engine.csproj" />
9+
<ProjectReference Include="..\SetupProcessFake\SetupProcessFake.csproj" />
910
<ProjectReference Include="..\TestHelper\TestHelper.csproj" />
1011
</ItemGroup>
1112

‎src/ServiceControlInstaller.Engine.UnitTests/Setup/SetupInstanceTests.cs‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
namespace ServiceControlInstaller.Engine.UnitTests.Setup;
2+
3+
using System;
4+
using System.Threading;
5+
using Engine.Setup;
6+
using NUnit.Framework;
7+
8+
[TestFixture]
9+
public class SetupInstanceTests
10+
{
11+
[Test]
12+
public void Should_not_throw_on_0_exit_code() => Assert.DoesNotThrow(() => InstanceSetup.Run(TestContext.CurrentContext.WorkDirectory, "SetupProcessFake.exe", "test", "", Timeout.Infinite));
13+
14+
[Test]
15+
public void Should_capture_and_rethrow_failures()
16+
{
17+
var ex = Assert.Throws<Exception>(() => InstanceSetup.Run(TestContext.CurrentContext.WorkDirectory, "SetupProcessFake.exe", "test", "fail", Timeout.Infinite));
18+
19+
Assert.That(ex.Message, Does.Contain("Fake exception"));
20+
}
21+
22+
[Test]
23+
public void Should_capture_and_rethrow_non_zero_exit_codes()
24+
{
25+
var ex = Assert.Throws<Exception>(() => InstanceSetup.Run(TestContext.CurrentContext.WorkDirectory, "SetupProcessFake.exe", "test", "non-zero-exit-code", Timeout.Infinite));
26+
27+
Assert.That(ex.Message, Does.Contain("returned a non-zero exit code: 3"));
28+
Assert.That(ex.Message, Does.Contain("Fake non zero exit code message"));
29+
}
30+
31+
[Test]
32+
public void Should_not_kill_the_process_if_wait_time_is_execeeded()
33+
{
34+
var process = InstanceSetup.Run(TestContext.CurrentContext.WorkDirectory, "SetupProcessFake.exe", "test", "delay", 10);
35+
36+
Assert.That(process.HasExited, Is.False);
37+
38+
process.Kill();
39+
process.WaitForExit();
40+
}
41+
}

‎src/ServiceControlInstaller.Engine/Setup/InstanceSetup.cs‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
using System;
44
using System.Diagnostics;
55
using System.IO;
6+
using System.Threading;
67
using Instances;
78

89
static class InstanceSetup
@@ -34,6 +35,13 @@ static void Run(string installPath, string exeName, string instanceName, bool sk
3435
args += " --skip-queue-creation";
3536
}
3637

38+
// we will wait "forever" since that is the most safe action right not. We will leave it up to the setup code in the instances to make sure it won't run forever.
39+
// If/when provide better UI experience we might revisit this and potentially find a way for the installer to control the timeout.
40+
Run(installPath, exeName, instanceName, args, Timeout.Infinite);
41+
}
42+
43+
internal static Process Run(string installPath, string exeName, string instanceName, string args, int timeout)
44+
{
3745
var processStartupInfo = new ProcessStartInfo
3846
{
3947
CreateNoWindow = true,
@@ -46,25 +54,17 @@ static void Run(string installPath, string exeName, string instanceName, bool sk
4654

4755
processStartupInfo.EnvironmentVariables.Add("INSTANCE_NAME", instanceName);
4856

49-
var p = Process.Start(processStartupInfo);
50-
if (p != null)
51-
{
52-
var error = p.StandardError.ReadToEnd();
53-
p.WaitForExit((int)TimeSpan.FromMinutes(1).TotalMilliseconds);
54-
if (!p.HasExited)
55-
{
56-
p.Kill();
57-
throw new TimeoutException($"Timed out waiting for {exeName} to perform setup. This usually indicates a configuration error.");
58-
}
57+
var p = Process.Start(processStartupInfo) ?? throw new Exception($"Attempt to launch {exeName} failed.");
5958

60-
if (p.ExitCode != 0)
61-
{
62-
throw new Exception($"{exeName} threw an error when performing setup. This typically indicates a configuration error. The error output from {exeName} was:\r\n {error}");
63-
}
64-
}
65-
else
59+
p.WaitForExit(timeout);
60+
61+
if (!p.HasExited || p.ExitCode == 0)
6662
{
67-
throw new Exception($"Attempt to launch {exeName} failed.");
63+
return p;
6864
}
65+
66+
var error = p.StandardError.ReadToEnd();
67+
68+
throw new Exception($"{exeName} returned a non-zero exit code: {p.ExitCode}. This typically indicates a configuration error. The error output was:\r\n {error}");
6969
}
7070
}

‎src/SetupProcessFake/Program.cs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
if (args.Any(a => a == "fail"))
2+
{
3+
throw new Exception("Fake exception");
4+
}
5+
6+
if (args.Any(a => a == "non-zero-exit-code"))
7+
{
8+
Console.Error.WriteLine("Fake non zero exit code message");
9+
10+
return 3;
11+
}
12+
13+
if (args.Any(a => a == "delay"))
14+
{
15+
#pragma warning disable CA2007 // Consider calling ConfigureAwait on the awaited task
16+
await Task.Delay(TimeSpan.FromSeconds(5));
17+
#pragma warning restore CA2007 // Consider calling ConfigureAwait on the awaited task
18+
}
19+
20+
return 0;

‎src/SetupProcessFake/SetupProcessFake.csproj‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<Project Sdk="Microsoft.NET.Sdk">
2+
3+
<PropertyGroup>
4+
<OutputType>Exe</OutputType>
5+
<TargetFramework>net8.0</TargetFramework>
6+
<ImplicitUsings>enable</ImplicitUsings>
7+
<Nullable>enable</Nullable>
8+
</PropertyGroup>
9+
10+
</Project>

0 commit comments

Comments
 (0)