From e1c1cf950d8272e4e90816a7cb7829ff7daaad01 Mon Sep 17 00:00:00 2001 From: Yihuang Yu Date: Sun, 21 Jun 2026 11:04:47 +0800 Subject: [PATCH] fix(up): add Service.containerName helper for explicit container_name ComposeUp had three places (getIPForRunningService, waitUntilServiceIsRunning, stopOldStuff) that hard-coded the container name as "\(projectName)-\(serviceName)" instead of reading service.container_name. When a compose file set an explicit container_name, the container started correctly (container run --name used the right value), but the post-start lookups waited for the wrong name and timed out after 30s. Extract a single Service.containerName(projectName:serviceName:) helper and replace all five call sites (four in ComposeUp, one in ComposeDown) with it. Follow-ups from review: - testUpWithExplicitContainerName now asserts the container has a network with an IPv4 address after up, which only happens if both waitUntilServiceIsRunning and getIPForRunningService resolve a real container. This catches a regression where the run-loop catch swallows wait timeouts (the container would still be running but have no networks attached). - Re-add the "Info: Using explicit container_name" log in configService that was dropped when centralizing the rule. --- .../Codable Structs/Service.swift | 6 +++ .../Commands/ComposeDown.swift | 8 +--- .../Commands/ComposeUp.swift | 39 +++++++++---------- .../ComposeUpTests.swift | 31 +++++++++++++++ 4 files changed, 56 insertions(+), 28 deletions(-) diff --git a/Sources/Container-Compose/Codable Structs/Service.swift b/Sources/Container-Compose/Codable Structs/Service.swift index 8e84cb09..eaeee3f0 100644 --- a/Sources/Container-Compose/Codable Structs/Service.swift +++ b/Sources/Container-Compose/Codable Structs/Service.swift @@ -303,4 +303,10 @@ public struct Service: Codable, Hashable { return sorted } + + /// Returns the container name, preferring an explicit `container_name` over the default pattern. + public func containerName(projectName: String, serviceName: String) -> String { + if let explicit = container_name { return explicit } + return "\(projectName)-\(serviceName)" + } } diff --git a/Sources/Container-Compose/Commands/ComposeDown.swift b/Sources/Container-Compose/Commands/ComposeDown.swift index 30240c65..75cdeaa5 100644 --- a/Sources/Container-Compose/Commands/ComposeDown.swift +++ b/Sources/Container-Compose/Commands/ComposeDown.swift @@ -121,13 +121,7 @@ public struct ComposeDown: AsyncParsableCommand { guard let projectName else { return } for (serviceName, service) in services { - // Respect explicit container_name, otherwise use default pattern - let containerName: String - if let explicitContainerName = service.container_name { - containerName = explicitContainerName - } else { - containerName = "\(projectName)-\(serviceName)" - } + let containerName = service.containerName(projectName: projectName, serviceName: serviceName) print("Stopping container: \(containerName)") diff --git a/Sources/Container-Compose/Commands/ComposeUp.swift b/Sources/Container-Compose/Commands/ComposeUp.swift index 97a16f96..f81ba079 100644 --- a/Sources/Container-Compose/Commands/ComposeUp.swift +++ b/Sources/Container-Compose/Commands/ComposeUp.swift @@ -156,7 +156,7 @@ public struct ComposeUp: AsyncParsableCommand, @unchecked Sendable { } // Stop Services - try await stopOldStuff(services.map({ $0.serviceName }), remove: true) + try await stopOldStuff(services, remove: true) // Process top-level networks // This creates named networks defined in the docker-compose.yml @@ -246,10 +246,10 @@ public struct ComposeUp: AsyncParsableCommand, @unchecked Sendable { return (entrypointFlag, positional) } - private func getIPForRunningService(_ serviceName: String) async throws -> String? { + private func getIPForRunningService(_ service: Service, serviceName: String) async throws -> String? { guard let projectName else { return nil } - let containerName = "\(projectName)-\(serviceName)" + let containerName = service.containerName(projectName: projectName, serviceName: serviceName) let client = ContainerClient() let container = try await client.get(id: containerName) @@ -263,13 +263,14 @@ public struct ComposeUp: AsyncParsableCommand, @unchecked Sendable { /// Repeatedly checks `container list -a` until the given container is listed as `running`. /// - Parameters: - /// - containerName: The exact name of the container (e.g. "Assignment-Manager-API-db"). + /// - service: The Service definition (needed to read `container_name`). + /// - serviceName: The service key in `docker-compose.yml`. /// - timeout: Max seconds to wait before failing. /// - interval: How often to poll (in seconds). /// - Returns: `true` if the container reached "running" state within the timeout. - private func waitUntilServiceIsRunning(_ serviceName: String, timeout: TimeInterval = 30, interval: TimeInterval = 0.5) async throws { + private func waitUntilServiceIsRunning(_ service: Service, serviceName: String, timeout: TimeInterval = 30, interval: TimeInterval = 0.5) async throws { guard let projectName else { return } - let containerName = "\(projectName)-\(serviceName)" + let containerName = service.containerName(projectName: projectName, serviceName: serviceName) let deadline = Date().addingTimeInterval(timeout) let client = ContainerClient() @@ -289,14 +290,14 @@ public struct ComposeUp: AsyncParsableCommand, @unchecked Sendable { ]) } - private func stopOldStuff(_ services: [String], remove: Bool) async throws { + private func stopOldStuff(_ services: [(serviceName: String, service: Service)], remove: Bool) async throws { guard let projectName else { return } - let containers = services.map { "\(projectName)-\($0)" } - for container in containers { - print("Stopping container: \(container)") + for (serviceName, service) in services { + let containerName = service.containerName(projectName: projectName, serviceName: serviceName) + print("Stopping container: \(containerName)") let client = ContainerClient() - guard let container = try? await client.get(id: container) else { continue } + guard let container = try? await client.get(id: containerName) else { continue } do { try await client.stop(id: container.id) @@ -315,8 +316,8 @@ public struct ComposeUp: AsyncParsableCommand, @unchecked Sendable { // MARK: Compose Top Level Functions - private mutating func updateEnvironmentWithServiceIP(_ serviceName: String) async throws { - let ip = try await getIPForRunningService(serviceName) + private mutating func updateEnvironmentWithServiceIP(_ service: Service, serviceName: String) async throws { + let ip = try await getIPForRunningService(service, serviceName: serviceName) self.containerIps[serviceName] = ip for (key, value) in environmentVariables.map({ ($0, $1) }) where value == serviceName { self.environmentVariables[key] = ip ?? value @@ -438,13 +439,9 @@ public struct ComposeUp: AsyncParsableCommand, @unchecked Sendable { } // Determine container name - let containerName: String - if let explicitContainerName = service.container_name { - containerName = explicitContainerName + let containerName = service.containerName(projectName: projectName, serviceName: serviceName) + if service.container_name != nil { print("Info: Using explicit container_name: \(containerName)") - } else { - // Default container name based on project and service name - containerName = "\(projectName)-\(serviceName)" } runCommandArgs.append("--name") runCommandArgs.append(containerName) @@ -638,8 +635,8 @@ public struct ComposeUp: AsyncParsableCommand, @unchecked Sendable { } do { - try await waitUntilServiceIsRunning(serviceName) - try await updateEnvironmentWithServiceIP(serviceName) + try await waitUntilServiceIsRunning(service, serviceName: serviceName) + try await updateEnvironmentWithServiceIP(service, serviceName: serviceName) } catch { print(error) } diff --git a/Tests/Container-Compose-DynamicTests/ComposeUpTests.swift b/Tests/Container-Compose-DynamicTests/ComposeUpTests.swift index 2d5b9877..f7b95bb3 100644 --- a/Tests/Container-Compose-DynamicTests/ComposeUpTests.swift +++ b/Tests/Container-Compose-DynamicTests/ComposeUpTests.swift @@ -375,6 +375,37 @@ struct ComposeUpTests { try? await stopInstance(location: upProject.base) } + @Test("up with explicit container_name starts the right container") + func testUpWithExplicitContainerName() async throws { + let explicitName = "container-compose-test-\(makeContainerName())" + let yaml = DockerComposeYamlFiles.dockerComposeYaml9(containerName: explicitName) + let project = try DockerComposeYamlFiles.copyYamlToTemporaryLocation(yaml: yaml) + + var composeUp = try ComposeUp.parse([ + "-d", "--cwd", project.base.path(percentEncoded: false), + ]) + try await composeUp.run() + + let client = ContainerClient() + let container = try? await client.get(id: explicitName) + #expect( + container != nil, + "Expected container \(explicitName) to exist after up, but get returned nil" + ) + #expect( + container?.status == .running, + "Expected container \(explicitName) to be running, got \(container?.status ?? .unknown)" + ) + + let resolvedIP = container?.networks.compactMap { $0.ipv4Address.address.description }.first + #expect( + resolvedIP != nil, + "Expected getIPForRunningService to resolve an IP for \(explicitName); got nil. A swallowed wait timeout would also produce a running container with no networks." + ) + + try? await stopInstance(location: project.base) + } + enum Errors: Error { case containerNotFound }