From 4743c135cba799d60631d847324878bd909e6849 Mon Sep 17 00:00:00 2001 From: GaTT Geng Date: Tue, 20 Oct 2026 16:32:35 +0800 Subject: [PATCH] fix: preserve on-failure restart retry counts --- CHANGELOG.md | 2 + README.md | 2 + README.zh-CN.md | 2 + docs/compose-field-matrix.md | 2 +- docs/roadmap.md | 2 + .../Internal/ContainerLifecycle.cs | 13 ++++-- src/ComposeSharp.Loader/ComposeFileLoader.cs | 12 +++++- .../Models/ComposeService.cs | 2 +- .../ComposeServiceIntegrationTests.cs | 42 +++++++++++++++++++ .../ComposeLoaderValidationTests.cs | 29 +++++++++++++ .../ContainerLifecycleTests.cs | 31 ++++++++++++++ 11 files changed, 132 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c79ac48..dbeac51 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,8 @@ This project follows [Semantic Versioning](https://semver.org/). ### Fixed +- Service-level `on-failure:N` restart policies preserve the Docker retry limit and reject malformed suffixes with contextual validation errors. + - `WaitAsync` collects container exit codes without concurrent dictionary writes and chooses its aggregate code deterministically by ordinal container ID. - `ListAsync` keeps projects with paused or restarting containers in the default listing, following Docker's non-`All` container results. - `StartAsync` reports Docker start failures and excludes retained one-off containers from dependency starts and readiness checks. diff --git a/README.md b/README.md index 20006d0..2a32fe0 100644 --- a/README.md +++ b/README.md @@ -107,6 +107,8 @@ The loader can retain fields that the engine does not yet apply. Consult the [Co ## What the engine does today +`restart: on-failure:N` applies the retry limit to Docker. Counts must be between 0 and 2147483647; zero or omission means unlimited retries. Malformed suffixes are rejected with source, service, and property diagnostics. + | Area | Current behavior | | --- | --- | | Project lifecycle | Creates project networks, creates/starts/removes labeled containers, lists project containers, removes project networks and optional volumes, and builds selected services through Docker Engine. | diff --git a/README.zh-CN.md b/README.zh-CN.md index de93a7e..04c342c 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -83,6 +83,8 @@ builder.Services.AddComposeSharp(); ## 当前能力与边界 +`restart: on-failure:N` 会将重试次数传给 Docker。次数范围为 0 到 2147483647；零或省略次数表示无限重试。非法后缀会被拒绝，错误包含源文件、服务和属性信息。 + | 能力 | 当前实现 | | --- | --- | | 项目生命周期 | 创建项目网络，创建/启动/删除带标签的容器，列举项目容器，按需删除网络和卷，并通过 Docker Engine 构建已选择的服务。 | diff --git a/docs/compose-field-matrix.md b/docs/compose-field-matrix.md index 991268a..3391215 100644 --- a/docs/compose-field-matrix.md +++ b/docs/compose-field-matrix.md @@ -26,7 +26,7 @@ ComposeSharp is an in-process SDK, not a Docker Compose CLI replacement. The loa | `environment`, `env_file` | `Environment`, `EnvFile` | Partial | Inline environment and values read from scalar or list-of-string `env_file` entries are passed to the container, and their original paths are retained for inspection. Long `env_file` syntax and advanced Compose environment semantics are not implemented. | | `ports` | `Ports` | Partial | Short `HOST:CONTAINER` string syntax creates a port binding when each container port has one mapping. Container-only entries expose the port but do not create a host binding. Multiple mappings for one container port, long syntax, host IP, ranges, and other per-port options are not modeled. | | `volumes` | `Volumes` | Partial | Short POSIX bind and named-volume strings are resolved and passed as binds. Long syntax, Windows drive-letter binds, and top-level volume driver/options are not applied. | -| `restart` | `Restart`, `RestartMaxRetries` | Partial | Docker restart policy name is applied. An `on-failure` retry count is parsed but not sent. | +| `restart` | `Restart`, `RestartMaxRetries` | Applied | Docker restart policy name and `on-failure:N` retry count are applied. Counts range from 0 to 2147483647; zero or omission means unlimited retries. Malformed suffixes are rejected with source/service/property diagnostics. Verified by `CreateAsync_AppliesRestartRetryCount` and focused loader/mapping tests. | | `healthcheck` | `Healthcheck` | Partial | Disable flag, list-form tests, supported interval/timeout/start-period values, and retries are sent in the create-container request. `start_interval` is not modeled. Durations accept .NET `TimeSpan` syntax or a single whole-number `ms`, `s`, `m`, or `h` unit; fractional and multi-component Compose durations are not accepted. A scalar `test` command is not converted to `CMD-SHELL`. | | `depends_on` | `DependsOn` | Partial | Selected services include their transitive dependencies and start in stable topological order; missing services and cycles fail with diagnostics. A dependency with an effective Docker health check, including an image-inherited check, must become healthy before its dependents start; otherwise it must be running. Readiness waits continue until ready, failure, cancellation, or the optional `DependencyWaitTimeoutSeconds` limit. Long-form conditions are not retained, and targeted lifecycle operations still do not start unrelated services. | | `networks` | `Networks` | Partial | The first declared service network becomes the container network mode. Per-network configuration and multi-network attachment are not applied. When a service declares no network and the project has custom networks, it attaches to the first custom network rather than an implicit default network. | diff --git a/docs/roadmap.md b/docs/roadmap.md index abb7394..d6a3168 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -39,6 +39,8 @@ Full Compose Specification parity in one release. Unsupported or partially appli ### Implemented contracts +Service-level `restart: on-failure:N` now preserves the Docker retry limit and rejects malformed suffixes with loader diagnostics; zero or omission means unlimited retries. + `GenerateAsync` now returns a normalized YAML snapshot of the loader's retained model alongside its summary. It supports explicit service selection and an output project name, without Docker inspection or file writes. This is a subset model snapshot, not complete Compose CLI configuration output; see the [generation and watch contract](generate-watch.md). `WatchAsync` now continuously monitors selected build contexts together, reports changes for each associated service, and propagates cancellation and watcher failures. It remains a notification API: starting, rebuilding, synchronizing, pruning, and interpreting `develop.watch` are outside its contract. diff --git a/src/ComposeSharp.Engine/Internal/ContainerLifecycle.cs b/src/ComposeSharp.Engine/Internal/ContainerLifecycle.cs index a80cdaa..f77b6df 100644 --- a/src/ComposeSharp.Engine/Internal/ContainerLifecycle.cs +++ b/src/ComposeSharp.Engine/Internal/ContainerLifecycle.cs @@ -479,11 +479,11 @@ private HostConfig BuildHostConfig(string projectName, ComposeProject project, S return result.Count == 0 ? null : result; } - private static RestartPolicy? BuildRestartPolicy(string? restart) + internal static RestartPolicy? BuildRestartPolicy(string? restart) { if (string.IsNullOrWhiteSpace(restart)) return null; - // "on-failure:N" splits off the max-retry suffix; only the policy kind is mapped here. - var normalized = restart.Split(':').First(); + var parts = restart.Split(':'); + var normalized = parts[0]; var kind = normalized.ToLowerInvariant() switch { "always" => RestartPolicyKind.Always, @@ -492,7 +492,12 @@ private HostConfig BuildHostConfig(string projectName, ComposeProject project, S "no" => RestartPolicyKind.No, _ => RestartPolicyKind.Undefined }; - return new RestartPolicy { Name = kind }; + var retries = 0; + if (parts.Length > 1 && (kind != RestartPolicyKind.OnFailure || parts.Length != 2 || + !int.TryParse(parts[1], System.Globalization.NumberStyles.None, + System.Globalization.CultureInfo.InvariantCulture, out retries))) + throw new ArgumentException("Restart retry count must use on-failure:N with N between 0 and 2147483647.", nameof(restart)); + return new RestartPolicy { Name = kind, MaximumRetryCount = retries }; } private static HealthConfig? BuildHealthcheck(ComposeHealthcheck? healthcheck) diff --git a/src/ComposeSharp.Loader/ComposeFileLoader.cs b/src/ComposeSharp.Loader/ComposeFileLoader.cs index aeba902..f499758 100644 --- a/src/ComposeSharp.Loader/ComposeFileLoader.cs +++ b/src/ComposeSharp.Loader/ComposeFileLoader.cs @@ -471,6 +471,16 @@ private static void ValidateService( // Stage 1: presence rules (image/build) and top-level field shapes. ValidateOptionalScalar(service, "image", $"{path}.image", context); ValidateBuild(service, path, context); + ValidateOptionalScalar(service, "restart", $"{path}.restart", context); + var restart = GetString(service, "restart"); + if (restart?.Contains(':') == true) + { + var parts = restart.Split(':'); + if (parts.Length != 2 || !parts[0].Equals("on-failure", StringComparison.OrdinalIgnoreCase) || + !int.TryParse(parts[1], NumberStyles.None, CultureInfo.InvariantCulture, out _)) + throw Validation(context, $"{path}.restart", + "restart retry count must use 'on-failure:N' with N between 0 and 2147483647."); + } var hasImage = TryGetValue(service, "image", out var image) && image is not null; var hasBuild = TryGetValue(service, "build", out var build) && build is not null; @@ -1138,7 +1148,7 @@ private ServiceDefinition ParseService( OomKillDisable: GetBool(map, "oom_kill_disable") ?? false ? 1L : null, OomScoreAdj: GetString(map, "oom_score_adj"), GroupAdd: GetStringList(map, "group_add"), - // `restart: on-failure:3` style keeps the retry count separately; the engine does not send it yet. + // Keep the suffix for callers inspecting the loaded model; the engine applies Restart verbatim. RestartMaxRetries: GetString(map, "restart")?.Contains(":") == true ? GetString(map, "restart")!.Split(':').Last() : null, diff --git a/src/ComposeSharp.Loader/Models/ComposeService.cs b/src/ComposeSharp.Loader/Models/ComposeService.cs index 8d5f3a2..c0c19c5 100644 --- a/src/ComposeSharp.Loader/Models/ComposeService.cs +++ b/src/ComposeSharp.Loader/Models/ComposeService.cs @@ -19,7 +19,7 @@ namespace ComposeSharp.Loader.Models; /// Develop: watch configuration is not interpreted. /// EnvFile: original entries kept for inspection; their values are already folded into Environment. /// StopSignal/StopGracePeriod/Sysctls/Logging/Platform/PullPolicy/Dns/DnsSearch/Links/CpuQuota/Annotations/OomScoreAdj: parsed only, not sent to Docker. -/// RestartMaxRetries: retry count stripped from an on-failure:N restart value; parsed but not sent. +/// RestartMaxRetries: retry count stripped from an on-failure:N restart value; the engine applies the count from Restart. Zero or omission means unlimited retries; counts must be between 0 and 2147483647. /// OomKillDisable: retained as 1 only when the YAML value is true; an explicit false is indistinguishable from omission. /// /// diff --git a/tests/ComposeSharp.IntegrationTests/ComposeServiceIntegrationTests.cs b/tests/ComposeSharp.IntegrationTests/ComposeServiceIntegrationTests.cs index 3b9a506..5131964 100644 --- a/tests/ComposeSharp.IntegrationTests/ComposeServiceIntegrationTests.cs +++ b/tests/ComposeSharp.IntegrationTests/ComposeServiceIntegrationTests.cs @@ -31,6 +31,48 @@ public class ComposeServiceIntegrationTests private static readonly SemaphoreSlim ImageGate = new(1, 1); private static bool _busyBoxReady; + [Theory] + [InlineData("on-failure", 0)] + [InlineData("on-failure:5", 5)] + public async Task CreateAsync_AppliesRestartRetryCount(string restart, int retries) + { + if (!DockerAvailable) return; + await EnsureBusyBoxAsync(); + var projectName = $"restart-retries-{Guid.NewGuid():N}"; + var directory = Path.Combine(Path.GetTempPath(), projectName); + Directory.CreateDirectory(directory); + File.WriteAllText(Path.Combine(directory, "compose.yaml"), + $"services:\n app:\n image: busybox:1.36\n restart: '{restart}'\n"); + var context = new ComposeProjectContext + { + ProjectName = projectName, + WorkingDirectory = directory, + ComposeFileName = "compose.yaml" + }; + var service = new ComposeService(); + try + { + await service.CreateAsync(context); + using var client = new DockerClientFactory().CreateClient(); + var containers = await client.Containers.ListContainersAsync(new ContainersListParameters + { + All = true, + Filters = new Dictionary> + { + ["label"] = new Dictionary { [$"com.docker.compose.project={projectName}"] = true } + } + }); + var container = await client.Containers.InspectContainerAsync(Assert.Single(containers).ID); + Assert.Equal(RestartPolicyKind.OnFailure, container.HostConfig.RestartPolicy.Name); + Assert.Equal(retries, container.HostConfig.RestartPolicy.MaximumRetryCount); + } + finally + { + try { await service.DownAsync(context); } + finally { Directory.Delete(directory, recursive: true); } + } + } + // Probes `docker info` once per test class; a missing CLI or unreachable daemon disables the Docker cases. private static bool CheckDockerAvailable() { diff --git a/tests/ComposeSharp.Tests/ComposeLoaderValidationTests.cs b/tests/ComposeSharp.Tests/ComposeLoaderValidationTests.cs index bbc0bde..4c0627b 100644 --- a/tests/ComposeSharp.Tests/ComposeLoaderValidationTests.cs +++ b/tests/ComposeSharp.Tests/ComposeLoaderValidationTests.cs @@ -245,6 +245,35 @@ public void LoadMerged_InvalidAppendedListItem_ReportsOverlaySource() exception, files.OverlayPath!, "api", "services.api.ports[1]", "short port mapping"); } + [Theory] + [InlineData("on-failure:")] + [InlineData("on-failure:-1")] + [InlineData("on-failure:+1")] + [InlineData("on-failure: 5")] + [InlineData("on-failure:1.5")] + [InlineData("on-failure:abc")] + [InlineData("on-failure:2147483648")] + [InlineData("on-failure:5:6")] + [InlineData("always:5")] + public void Load_InvalidRestartSuffix_ReportsFullContext(string restart) + { + using var files = new ComposeFiles($"services:\n api:\n image: app\n restart: '{restart}'\n"); + var exception = Assert.Throws(() => files.Load()); + AssertServiceDiagnostic(exception, files.PrimaryPath, "api", "services.api.restart", "on-failure:N"); + } + + [Theory] + [InlineData("on-failure")] + [InlineData("on-failure:0")] + [InlineData("on-failure:5")] + [InlineData("on-failure:2147483647")] + public void Load_ValidRestartSuffix_PreservesValue(string restart) + { + using var files = new ComposeFiles($"services:\n api:\n image: app\n restart: '{restart}'\n"); + var project = new ComposeFileLoader().Load(files.DirectoryPath, "compose.yaml"); + Assert.Equal(restart, Assert.Single(project.Services).Restart); + } + // Asserts project-level diagnostics carry the source file and root property path but no service name. private static void AssertProjectDiagnostic( ComposeValidationException exception, diff --git a/tests/ComposeSharp.Tests/ContainerLifecycleTests.cs b/tests/ComposeSharp.Tests/ContainerLifecycleTests.cs index c7c66a3..cf7c62f 100644 --- a/tests/ComposeSharp.Tests/ContainerLifecycleTests.cs +++ b/tests/ComposeSharp.Tests/ContainerLifecycleTests.cs @@ -10,6 +10,37 @@ namespace ComposeSharp.Tests; public class ContainerLifecycleTests { + [Theory] + [InlineData("on-failure", RestartPolicyKind.OnFailure, 0)] + [InlineData("on-failure:5", RestartPolicyKind.OnFailure, 5)] + [InlineData("on-failure:0", RestartPolicyKind.OnFailure, 0)] + [InlineData("on-failure:2147483647", RestartPolicyKind.OnFailure, int.MaxValue)] + [InlineData("always", RestartPolicyKind.Always, 0)] + [InlineData("unless-stopped", RestartPolicyKind.UnlessStopped, 0)] + [InlineData("no", RestartPolicyKind.No, 0)] + public void BuildRestartPolicy_PreservesRetryCount(string restart, RestartPolicyKind kind, int retries) + { + var policy = ContainerLifecycle.BuildRestartPolicy(restart); + Assert.NotNull(policy); + Assert.Equal(kind, policy.Name); + Assert.Equal(retries, policy.MaximumRetryCount); + } + + [Theory] + [InlineData("on-failure:")] + [InlineData("on-failure:-1")] + [InlineData("on-failure:+1")] + [InlineData("on-failure: 5")] + [InlineData("on-failure:1.5")] + [InlineData("on-failure:abc")] + [InlineData("on-failure:2147483648")] + [InlineData("on-failure:5:6")] + [InlineData("always:5")] + public void BuildRestartPolicy_RejectsInvalidSuffix(string restart) + { + Assert.Throws(() => ContainerLifecycle.BuildRestartPolicy(restart)); + } + [Fact] public void FindByContainerNumber_UsesLabelInsteadOfListPosition() {