Skip to content

Commit c97e9f6

Browse files
mariotoffiaclaude
mariotoffia
and
claude
authored
fix(#283): make ExecuteOnRunning run each command once, after readiness (#345)
ExecuteOnRunning was effectively broken in v3 due to three defects: 1. Semantic regression: the params string[] was joined into ONE command (string.Join(" ", ...)), so passing an array of full commands produced a single malformed command. Restored the v2 contract: each element is executed as a separate command. 2. Double execution + wrong ordering: hooks ran once in ContainerService.StartAsync AND again in the builder, both BEFORE wait conditions. The builder is now the single owner of Running lifecycle hooks: StartAsync no longer runs them; CopyToOnStart runs right after start, and ExecuteOnRunning runs AFTER the wait conditions so commands fire only once the container is actually ready (e.g. after WaitForMessageInLog). 3. Silent failure: exec errors were swallowed and only logged. Execute hooks now propagate failures (DriverException) so callers see them. Unified the no-links and linked-container paths via RunPostStartAsync (CopyTo -> wait conditions -> Execute). The dispose-time Execute path (ExecuteOnDisposing) also now runs per-element. Tests: each command runs separately and exactly once; ExecuteOnRunning runs after wait conditions; a failing command propagates. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 7711cfa commit c97e9f6

4 files changed

Lines changed: 138 additions & 13 deletions

File tree

‎FluentDocker.Tests/CoreTests/BuilderTests/BuilderContainerTests.LifecycleExec.cs‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Threading;
4+
using System.Threading.Tasks;
5+
using FluentDocker.Builders;
6+
using FluentDocker.Common;
7+
using FluentDocker.Drivers;
8+
using FluentDocker.Model.Drivers;
9+
using Moq;
10+
using Xunit;
11+
12+
namespace FluentDocker.Tests.CoreTests.BuilderTests
13+
{
14+
/// <summary>
15+
/// Tests for ExecuteOnRunning lifecycle execution (issue #283): each command runs as a
16+
/// separate command, exactly once, AFTER wait conditions, and failures propagate.
17+
/// </summary>
18+
public partial class BuilderContainerTests
19+
{
20+
private void SetupLifecycleBuild() =>
21+
MockPack
22+
.SetupContainerCreate()
23+
.SetupContainerStart()
24+
.SetupContainerInspect(running: true)
25+
.SetupContainerStop()
26+
.SetupContainerRemove();
27+
28+
[Fact]
29+
public async Task ExecuteOnRunning_RunsEachCommandAsSeparateCommand_ExactlyOnce()
30+
{
31+
SetupLifecycleBuild();
32+
33+
var execCommands = new List<string>();
34+
MockPack.ContainerDriver
35+
.Setup(d => d.ExecAsync(
36+
It.IsAny<DriverContext>(), It.IsAny<string>(),
37+
It.IsAny<ExecConfig>(), It.IsAny<CancellationToken>()))
38+
.Callback<DriverContext, string, ExecConfig, CancellationToken>(
39+
(_, _, cfg, _) => execCommands.Add(string.Join(" ", cfg.Command)))
40+
.ReturnsAsync(CommandResponse<ExecResult>.Ok(new ExecResult { StdOut = "", ExitCode = 0 }));
41+
42+
using var results = await new Builder()
43+
.WithinDriver(DriverId, Kernel)
44+
.UseContainer(c => c
45+
.UseImage("alpine")
46+
.ExecuteOnRunning("first", "second"))
47+
.BuildAsync(cancellationToken: TestContext.Current.CancellationToken);
48+
49+
// Each array element runs as its own command (not joined) and exactly once (no double-exec).
50+
Assert.Equal(new[] { "first", "second" }, execCommands);
51+
}
52+
53+
[Fact]
54+
public async Task ExecuteOnRunning_RunsAfterWaitConditions()
55+
{
56+
SetupLifecycleBuild();
57+
58+
var order = new List<string>();
59+
MockPack.ContainerDriver
60+
.Setup(d => d.ExecAsync(
61+
It.IsAny<DriverContext>(), It.IsAny<string>(),
62+
It.IsAny<ExecConfig>(), It.IsAny<CancellationToken>()))
63+
.Callback(() => order.Add("exec"))
64+
.ReturnsAsync(CommandResponse<ExecResult>.Ok(new ExecResult { StdOut = "", ExitCode = 0 }));
65+
66+
using var results = await new Builder()
67+
.WithinDriver(DriverId, Kernel)
68+
.UseContainer(c => c
69+
.UseImage("alpine")
70+
.Wait((_, _) => { order.Add("wait"); return -1; })
71+
.ExecuteOnRunning("after-ready"))
72+
.BuildAsync(cancellationToken: TestContext.Current.CancellationToken);
73+
74+
Assert.Equal(new[] { "wait", "exec" }, order);
75+
}
76+
77+
[Fact]
78+
public async Task ExecuteOnRunning_FailingCommand_PropagatesError()
79+
{
80+
SetupLifecycleBuild();
81+
82+
MockPack.ContainerDriver
83+
.Setup(d => d.ExecAsync(
84+
It.IsAny<DriverContext>(), It.IsAny<string>(),
85+
It.IsAny<ExecConfig>(), It.IsAny<CancellationToken>()))
86+
.ReturnsAsync(CommandResponse<ExecResult>.Fail("exec boom", "CONTAINER_EXEC_FAILED"));
87+
88+
await Assert.ThrowsAsync<DriverException>(async () =>
89+
await new Builder()
90+
.WithinDriver(DriverId, Kernel)
91+
.UseContainer(c => c
92+
.UseImage("alpine")
93+
.ExecuteOnRunning("will-fail"))
94+
.BuildAsync(cancellationToken: TestContext.Current.CancellationToken));
95+
}
96+
}
97+
}

‎FluentDocker/Builders/ContainerBuilder.WaitConditions.cs‎

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -103,24 +103,49 @@ internal async Task ExecuteDeferredWaitConditionsAsync(CancellationToken cancell
103103
return;
104104

105105
_waitConditionsExecuted = true;
106-
await ExecuteWaitConditionsAsync(_pendingService, cancellationToken).ConfigureAwait(false);
106+
await RunPostStartAsync(_pendingService, cancellationToken).ConfigureAwait(false);
107107
}
108108

109-
private async Task ExecuteLifecycleHooksAsync(
110-
Services.Impl.ContainerService service, ServiceRunningState state,
109+
/// <summary>
110+
/// Runs the post-start sequence for a started container: setup hooks (CopyToOnStart) first,
111+
/// then wait conditions, then ExecuteOnRunning commands. Running Execute hooks AFTER the wait
112+
/// conditions means commands fire only once the container is actually ready (issue #283).
113+
/// </summary>
114+
internal async Task RunPostStartAsync(
115+
Services.Impl.ContainerService service, CancellationToken cancellationToken)
116+
{
117+
await RunRunningLifecycleHooksAsync(service, executeCommands: false, cancellationToken).ConfigureAwait(false);
118+
await ExecuteWaitConditionsAsync(service, cancellationToken).ConfigureAwait(false);
119+
await RunRunningLifecycleHooksAsync(service, executeCommands: true, cancellationToken).ConfigureAwait(false);
120+
}
121+
122+
/// <summary>
123+
/// Runs Running-triggered lifecycle hooks. When <paramref name="executeCommands"/> is false,
124+
/// only setup hooks (CopyToOnStart) run; when true, only ExecuteOnRunning commands run.
125+
/// Each element of an Execute hook's command array is executed as a separate command,
126+
/// and failures propagate (the v2 contract) rather than being silently swallowed.
127+
/// </summary>
128+
private async Task RunRunningLifecycleHooksAsync(
129+
Services.Impl.ContainerService service, bool executeCommands,
111130
CancellationToken cancellationToken)
112131
{
113-
var hooks = _lifecycleHooks.Where(h => h.TriggerState == state);
114-
foreach (var hook in hooks)
132+
foreach (var hook in _lifecycleHooks)
115133
{
134+
if (hook.TriggerState != ServiceRunningState.Running)
135+
continue;
136+
116137
switch (hook.Type)
117138
{
118-
case LifecycleHookType.CopyTo:
139+
case LifecycleHookType.CopyTo when !executeCommands:
119140
await service.CopyToAsync(hook.ContainerPath,
120141
File.ReadAllBytes(hook.HostPath), cancellationToken).ConfigureAwait(false);
121142
break;
122-
case LifecycleHookType.Execute:
123-
await service.ExecuteAsync(string.Join(" ", hook.Command), cancellationToken).ConfigureAwait(false);
143+
case LifecycleHookType.Execute when executeCommands:
144+
if (hook.Command != null)
145+
{
146+
foreach (var command in hook.Command)
147+
await service.ExecuteAsync(command, cancellationToken).ConfigureAwait(false);
148+
}
124149
break;
125150
}
126151
}

‎FluentDocker/Builders/ContainerBuilder.cs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -469,9 +469,8 @@ public async Task<IServiceAsync> ExecuteAsync(CancellationToken cancellationToke
469469
{
470470
await service.StartAsync(cancellationToken).ConfigureAwait(false);
471471
await WaitForContainerRunningAsync(driver, context, response.Data.Id, cancellationToken).ConfigureAwait(false);
472-
await ExecuteLifecycleHooksAsync(service, ServiceRunningState.Running, cancellationToken).ConfigureAwait(false);
473-
await ExecuteWaitConditionsAsync(service, cancellationToken).ConfigureAwait(false);
474472
_waitConditionsExecuted = true;
473+
await RunPostStartAsync(service, cancellationToken).ConfigureAwait(false);
475474
}
476475
catch (Exception ex)
477476
{

‎FluentDocker/Services/Impl/ContainerService.cs‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,9 @@ public async Task StartAsync(CancellationToken cancellationToken = default)
132132

133133
UpdateState(ServiceRunningState.Running);
134134
await ExecuteHooksAsync(ServiceRunningState.Running).ConfigureAwait(false);
135-
await ExecuteLifecycleHooksAsync(ServiceRunningState.Running, cancellationToken).ConfigureAwait(false);
135+
// Running lifecycle hooks (CopyToOnStart / ExecuteOnRunning) are orchestrated by the
136+
// builder so they run exactly once and Execute hooks fire AFTER wait conditions. They
137+
// are intentionally NOT run here to avoid double execution and premature ordering.
136138
}
137139

138140
public async Task PauseAsync(CancellationToken cancellationToken = default)
@@ -428,9 +430,11 @@ private async Task ExecuteLifecycleHooksAsync(ServiceRunningState state, Cancell
428430
break;
429431

430432
case LifecycleHookType.Execute:
431-
if (hook.Command?.Length > 0)
433+
// Each element is a separate command (matches the v2 contract).
434+
if (hook.Command != null)
432435
{
433-
await ExecuteAsync(string.Join(" ", hook.Command), cancellationToken).ConfigureAwait(false);
436+
foreach (var command in hook.Command)
437+
await ExecuteAsync(command, cancellationToken).ConfigureAwait(false);
434438
}
435439
break;
436440
}

0 commit comments

Comments
 (0)
⚡