From 926b3a3a23ca2c95df3f150ca30b5c2ace708408 Mon Sep 17 00:00:00 2001 From: Raj Date: Thu, 5 Mar 2026 17:53:47 -0800 Subject: [PATCH] Add build.cpus and build.memory system properties (#1293) --- Sources/ContainerCommands/BuildCommand.swift | 4 +- .../Builder/BuilderStart.swift | 40 ++++++++--------- .../System/Property/PropertySet.swift | 20 ++++----- .../ContainerPersistence/DefaultsStore.swift | 18 ++++++++ .../ContainerAPIService/Client/Parser.swift | 16 +++++-- .../ContainerAPIClientTests/ParserTest.swift | 45 +++++++++++++++++++ 6 files changed, 108 insertions(+), 35 deletions(-) diff --git a/Sources/ContainerCommands/BuildCommand.swift b/Sources/ContainerCommands/BuildCommand.swift index 6e9f7387..f9f8f3bc 100644 --- a/Sources/ContainerCommands/BuildCommand.swift +++ b/Sources/ContainerCommands/BuildCommand.swift @@ -67,7 +67,7 @@ extension Application { }() @Option(name: .shortAndLong, help: "Number of CPUs to allocate to the builder container") - var cpus: Int64 = 2 + var cpus: Int64? @Option(name: .shortAndLong, help: ArgumentHelp("Path to Dockerfile", valueName: "path")) var file: String? @@ -79,7 +79,7 @@ extension Application { name: .shortAndLong, help: "Amount of builder container memory (1MiByte granularity), with optional K, M, G, T, or P suffix" ) - var memory: String = "2048MB" + var memory: String? @Flag(name: .long, help: "Do not use cache") var noCache: Bool = false diff --git a/Sources/ContainerCommands/Builder/BuilderStart.swift b/Sources/ContainerCommands/Builder/BuilderStart.swift index 855d8b4c..7baa82c7 100644 --- a/Sources/ContainerCommands/Builder/BuilderStart.swift +++ b/Sources/ContainerCommands/Builder/BuilderStart.swift @@ -29,6 +29,9 @@ import TerminalProgress extension Application { public struct BuilderStart: AsyncLoggableCommand { + static let defaultCPUs = 2 + static let defaultMemoryInBytes: UInt64 = 2048.mib() + public static var configuration: CommandConfiguration { var config = CommandConfiguration() config.commandName = "start" @@ -37,13 +40,13 @@ extension Application { } @Option(name: .shortAndLong, help: "Number of CPUs to allocate to the builder container") - var cpus: Int64 = 2 + var cpus: Int64? @Option( name: .shortAndLong, help: "Amount of builder container memory (1MiByte granularity), with optional K, M, G, T, or P suffix" ) - var memory: String = "2048MB" + var memory: String? @OptionGroup public var dns: Flags.DNS @@ -135,23 +138,16 @@ extension Application { // Check if we need to recreate the builder due to different image let imageChanged = existingImage != builderImage - let cpuChanged = { - if let cpus { - if existingResources.cpus != cpus { - return true - } - } - return false - }() - let memChanged = try { - if let memory { - let memoryInBytes = try Parser.resources(cpus: nil, memory: memory).memoryInBytes - if existingResources.memoryInBytes != memoryInBytes { - return true - } - } - return false - }() + let resolvedResources = try Parser.resources( + cpus: cpus, + memory: memory, + cpuPropertyKey: .defaultBuildCPUs, + memoryPropertyKey: .defaultBuildMemory, + defaultCPUs: Self.defaultCPUs, + defaultMemoryInBytes: Self.defaultMemoryInBytes + ) + let cpuChanged = existingResources.cpus != resolvedResources.cpus + let memChanged = existingResources.memoryInBytes != resolvedResources.memoryInBytes let dnsChanged = { if !dnsNameservers.isEmpty { return existingDNS?.nameservers != dnsNameservers @@ -241,7 +237,11 @@ extension Application { let resources = try Parser.resources( cpus: cpus, - memory: memory + memory: memory, + cpuPropertyKey: .defaultBuildCPUs, + memoryPropertyKey: .defaultBuildMemory, + defaultCPUs: Self.defaultCPUs, + defaultMemoryInBytes: Self.defaultMemoryInBytes ) var config = ContainerConfiguration(id: Builder.builderContainerId, image: imageDesc, process: processConfig) diff --git a/Sources/ContainerCommands/System/Property/PropertySet.swift b/Sources/ContainerCommands/System/Property/PropertySet.swift index 58f0520e..de79b296 100644 --- a/Sources/ContainerCommands/System/Property/PropertySet.swift +++ b/Sources/ContainerCommands/System/Property/PropertySet.swift @@ -51,6 +51,16 @@ extension Application { throw ContainerizationError(.invalidArgument, message: "invalid boolean value: \(value)") } DefaultsStore.setBool(value: boolValue, key: key) + case .defaultBuildCPUs, .defaultContainerCPUs: + guard let cpuCount = Int(value), cpuCount > 0 else { + throw ContainerizationError(.invalidArgument, message: "invalid CPU count: \(value)") + } + DefaultsStore.set(value: value, key: key) + case .defaultBuildMemory, .defaultContainerMemory: + guard let memoryMiB = try? Parser.memoryStringAsMiB(value), memoryMiB > 0 else { + throw ContainerizationError(.invalidArgument, message: "invalid memory value: \(value)") + } + DefaultsStore.set(value: value, key: key) case .defaultDNSDomain, .defaultRegistryDomain: guard Parser.isValidDomainName(value) else { throw ContainerizationError(.invalidArgument, message: "invalid domain name: \(value)") @@ -69,16 +79,6 @@ extension Application { } DefaultsStore.set(value: value, key: key) return - case .defaultContainerCPUs: - guard let cpuCount = Int(value), cpuCount > 0 else { - throw ContainerizationError(.invalidArgument, message: "invalid CPU count: \(value)") - } - DefaultsStore.set(value: value, key: key) - case .defaultContainerMemory: - guard let memoryMiB = try? Parser.memoryStringAsMiB(value), memoryMiB > 0 else { - throw ContainerizationError(.invalidArgument, message: "invalid memory value: \(value)") - } - DefaultsStore.set(value: value, key: key) case .defaultSubnet: guard (try? CIDRv4(value)) != nil else { throw ContainerizationError(.invalidArgument, message: "invalid CIDRv4 address: \(value)") diff --git a/Sources/ContainerPersistence/DefaultsStore.swift b/Sources/ContainerPersistence/DefaultsStore.swift index cb92037a..220e5267 100644 --- a/Sources/ContainerPersistence/DefaultsStore.swift +++ b/Sources/ContainerPersistence/DefaultsStore.swift @@ -24,6 +24,8 @@ public enum DefaultsStore { public enum Keys: String { case buildRosetta = "build.rosetta" + case defaultBuildCPUs = "build.cpus" + case defaultBuildMemory = "build.memory" case defaultContainerCPUs = "container.cpus" case defaultContainerMemory = "container.memory" case defaultDNSDomain = "dns.domain" @@ -71,6 +73,8 @@ public enum DefaultsStore { public static func allValues() -> [DefaultsStoreValue] { let allKeys: [(Self.Keys, (Self.Keys) -> Any?)] = [ (.buildRosetta, { Self.getBool(key: $0) }), + (.defaultBuildCPUs, { Self.getOptional(key: $0) }), + (.defaultBuildMemory, { Self.getOptional(key: $0) }), (.defaultContainerCPUs, { Self.getOptional(key: $0) }), (.defaultContainerMemory, { Self.getOptional(key: $0) }), (.defaultBuilderImage, { Self.get(key: $0) }), @@ -126,6 +130,10 @@ extension DefaultsStore.Keys { switch self { case .buildRosetta: return "Build amd64 images on arm64 using Rosetta, instead of QEMU." + case .defaultBuildCPUs: + return "If defined, the default number of CPUs to allocate to the builder container." + case .defaultBuildMemory: + return "If defined, the default amount of memory to allocate to the builder container." case .defaultContainerCPUs: return "If defined, the default number of CPUs to allocate to a container." case .defaultContainerMemory: @@ -153,6 +161,10 @@ extension DefaultsStore.Keys { switch self { case .buildRosetta: return Bool.self + case .defaultBuildCPUs: + return String.self + case .defaultBuildMemory: + return String.self case .defaultContainerCPUs: return String.self case .defaultContainerMemory: @@ -181,6 +193,12 @@ extension DefaultsStore.Keys { case .buildRosetta: // This is a boolean key, not used with the string get() method return "true" + case .defaultBuildCPUs: + // This key is read with getOptional(), not get(); this value is never used + return "2" + case .defaultBuildMemory: + // This key is read with getOptional(), not get(); this value is never used + return "2048MB" case .defaultContainerCPUs: // This key is read with getOptional(), not get(); this value is never used return "4" diff --git a/Sources/Services/ContainerAPIService/Client/Parser.swift b/Sources/Services/ContainerAPIService/Client/Parser.swift index 87b380a4..47752d0b 100644 --- a/Sources/Services/ContainerAPIService/Client/Parser.swift +++ b/Sources/Services/ContainerAPIService/Client/Parser.swift @@ -87,18 +87,28 @@ public struct Parser { try .init(from: platform) } - public static func resources(cpus: Int64?, memory: String?) throws -> ContainerConfiguration.Resources { + public static func resources( + cpus: Int64?, + memory: String?, + cpuPropertyKey: DefaultsStore.Keys = .defaultContainerCPUs, + memoryPropertyKey: DefaultsStore.Keys = .defaultContainerMemory, + defaultCPUs: Int = 4, + defaultMemoryInBytes: UInt64 = 1024.mib() + ) throws -> ContainerConfiguration.Resources { var resource = ContainerConfiguration.Resources() + resource.cpus = defaultCPUs + resource.memoryInBytes = defaultMemoryInBytes + if let cpus { resource.cpus = Int(cpus) - } else if let cpuStr = DefaultsStore.getOptional(key: .defaultContainerCPUs), + } else if let cpuStr = DefaultsStore.getOptional(key: cpuPropertyKey), let cpuVal = Int(cpuStr), cpuVal > 0 { resource.cpus = cpuVal } if let memory { resource.memoryInBytes = try Parser.memoryStringAsMiB(memory).mib() - } else if let memStr = DefaultsStore.getOptional(key: .defaultContainerMemory) { + } else if let memStr = DefaultsStore.getOptional(key: memoryPropertyKey) { resource.memoryInBytes = try Parser.memoryStringAsMiB(memStr).mib() } return resource diff --git a/Tests/ContainerAPIClientTests/ParserTest.swift b/Tests/ContainerAPIClientTests/ParserTest.swift index e8d005d6..1905443e 100644 --- a/Tests/ContainerAPIClientTests/ParserTest.swift +++ b/Tests/ContainerAPIClientTests/ParserTest.swift @@ -1047,6 +1047,34 @@ struct ParserTest { // MARK: - Parser.resources + @Test + func testResourcesCustomDefaults() throws { + let result = try Parser.resources( + cpus: nil, memory: nil, + cpuPropertyKey: .defaultBuildCPUs, memoryPropertyKey: .defaultBuildMemory, + defaultCPUs: 2, defaultMemoryInBytes: 2048.mib() + ) + #expect(result.cpus == 2) + #expect(result.memoryInBytes == 2048.mib()) + } + + @Test + func testResourcesBuildPropertyLookup() throws { + DefaultsStore.set(value: "8", key: .defaultBuildCPUs) + DefaultsStore.set(value: "4g", key: .defaultBuildMemory) + defer { + DefaultsStore.unset(key: .defaultBuildCPUs) + DefaultsStore.unset(key: .defaultBuildMemory) + } + let result = try Parser.resources( + cpus: nil, memory: nil, + cpuPropertyKey: .defaultBuildCPUs, memoryPropertyKey: .defaultBuildMemory, + defaultCPUs: 2, defaultMemoryInBytes: 2048.mib() + ) + #expect(result.cpus == 8) + #expect(result.memoryInBytes == 4096.mib()) + } + @Test func testResourcesCPUsFromProperty() throws { DefaultsStore.set(value: "8", key: .defaultContainerCPUs) @@ -1075,4 +1103,21 @@ struct ParserTest { #expect(result.cpus == 1) #expect(result.memoryInBytes == 256.mib()) } + + @Test + func testResourcesPropertyKeysAreIsolated() throws { + DefaultsStore.set(value: "16", key: .defaultContainerCPUs) + DefaultsStore.set(value: "8g", key: .defaultContainerMemory) + defer { + DefaultsStore.unset(key: .defaultContainerCPUs) + DefaultsStore.unset(key: .defaultContainerMemory) + } + let result = try Parser.resources( + cpus: nil, memory: nil, + cpuPropertyKey: .defaultBuildCPUs, memoryPropertyKey: .defaultBuildMemory, + defaultCPUs: 2, defaultMemoryInBytes: 2048.mib() + ) + #expect(result.cpus == 2) + #expect(result.memoryInBytes == 2048.mib()) + } }