Add idle container configuration for kamal-proxy#1800
Add idle container configuration for kamal-proxy#1800martijnenco wants to merge 1 commit intobasecamp:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Exposes kamal-proxy’s idle container (scale-to-zero) feature through Kamal’s deploy configuration, including deploy-time flags and automatic Docker socket mounting for the proxy container.
Changes:
- Add
proxy.idle.timeout/proxy.idle.wake_timeoutconfig, anidle?helper, and pass--idle-timeout/--idle-wake-timeouttokamal-proxy deploy. - Add
proxy.run.docker_socketand attempt to auto-mount/var/run/docker.sockwhen idle is configured. - Add validation/docs/tests for the new settings.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
lib/kamal/configuration/proxy.rb |
Adds idle? and includes idle flags in deploy_options. |
lib/kamal/configuration/proxy/run.rb |
Adds Docker socket mount logic via docker_socket?. |
lib/kamal/configuration/validator/proxy.rb |
Adds validation for idle config values. |
lib/kamal/configuration.rb |
Adds any_role_use_proxy_idle? helper used by proxy run config. |
lib/kamal/configuration/docs/proxy.yml |
Documents idle and run.docker_socket settings. |
test/configuration/proxy_test.rb |
Adds tests for idle parsing and validation. |
test/commands/app_test.rb |
Adds a test ensuring idle flags are included in deploy command output. |
test/commands/proxy_test.rb |
Adds tests for Docker socket mount behavior under proxy run configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if config["idle"]["timeout"].present? && !config["idle"]["timeout"].is_a?(Integer) | ||
| error "Idle timeout must be an integer (seconds)" | ||
| end | ||
|
|
||
| if config["idle"]["wake_timeout"].present? && !config["idle"]["wake_timeout"].is_a?(Integer) | ||
| error "Idle wake timeout must be an integer (seconds)" |
There was a problem hiding this comment.
The new idle validation only checks that the values are Integers, but it does not enforce the documented/PR-stated requirement that idle.timeout and idle.wake_timeout be positive (e.g., 0 or negative values would pass and produce "0s"/"-5s" durations later). Consider validating > 0 (and possibly adding a targeted error message) for any configured idle timeout values.
| if config["idle"]["timeout"].present? && !config["idle"]["timeout"].is_a?(Integer) | |
| error "Idle timeout must be an integer (seconds)" | |
| end | |
| if config["idle"]["wake_timeout"].present? && !config["idle"]["wake_timeout"].is_a?(Integer) | |
| error "Idle wake timeout must be an integer (seconds)" | |
| if config["idle"]["timeout"].present? | |
| if !config["idle"]["timeout"].is_a?(Integer) | |
| error "Idle timeout must be an integer (seconds)" | |
| elsif config["idle"]["timeout"] <= 0 | |
| error "Idle timeout must be a positive integer (seconds)" | |
| end | |
| end | |
| if config["idle"]["wake_timeout"].present? | |
| if !config["idle"]["wake_timeout"].is_a?(Integer) | |
| error "Idle wake timeout must be an integer (seconds)" | |
| elsif config["idle"]["wake_timeout"] <= 0 | |
| error "Idle wake timeout must be a positive integer (seconds)" | |
| end |
| end | ||
|
|
||
| def idle? | ||
| proxy_config.dig("idle", "timeout").present? |
There was a problem hiding this comment.
idle? only checks idle.timeout. If a user configures only idle.wake_timeout, idle? will be false (which also affects any_role_use_proxy_idle?) even though deploy_options may still emit an idle-related flag. Consider either treating wake_timeout as enabling idle as well, or validating that wake_timeout cannot be set without timeout.
| proxy_config.dig("idle", "timeout").present? | |
| idle_config = proxy_config["idle"] | |
| idle_config.is_a?(Hash) && (idle_config["timeout"].present? || idle_config["wake_timeout"].present?) |
| "log-response-header": proxy_config.dig("logging", "response_headers"), | ||
| "idle-timeout": seconds_duration(proxy_config.dig("idle", "timeout")), | ||
| "idle-wake-timeout": seconds_duration(proxy_config.dig("idle", "wake_timeout")), | ||
| "error-pages": error_pages |
There was a problem hiding this comment.
deploy_options will include idle-wake-timeout whenever idle.wake_timeout is set, even if idle.timeout is missing/disabled. That can produce a deploy command with a wake-timeout flag but no idle-timeout, and (depending on how the proxy container is started) without Docker socket access. Consider only emitting idle-wake-timeout when idle.timeout is configured, or enforce the dependency in validation.
| def docker_socket? | ||
| run_config.fetch("docker_socket", config.any_role_use_proxy_idle?) | ||
| end |
There was a problem hiding this comment.
The Docker socket auto-mount is implemented via Proxy::Run#docker_socket?, but Proxy::Run is only instantiated/used when a per-host proxy.run config exists. When proxy.run is not configured (the default path in Kamal::Commands::Proxy#run uses the boot-file pipeline), idle can be enabled via proxy.idle but the proxy container still won't get the Docker socket mount. If the intent is that setting proxy.idle alone enables scale-to-zero, this needs to also affect the boot-mode run options (or the docs should explicitly require proxy.run).
| metrics_port: 9090 # Port for Prometheus metrics | ||
| debug: true # Debug logging (default: false) | ||
| log_max_size: "30m" # Maximum log file size (default: "10m") | ||
| docker_socket: true # Mount the Docker socket (default: false, or true if idle is used) |
There was a problem hiding this comment.
The comment for run.docker_socket says the default is "false, or true if idle is used", but the current implementation only defaults to true when the proxy is started via a proxy.run config (i.e., when Proxy::Run is used). If the proxy is started via the boot-file path (no proxy.run), proxy.idle currently won’t imply a Docker socket mount. Please either adjust the docs to reflect this limitation or update the boot-mode run options to match the documented default.
| docker_socket: true # Mount the Docker socket (default: false, or true if idle is used) | |
| docker_socket: true # Mount the Docker socket (default: false; may default to true with idle when using proxy.run) |
| "docker run --name kamal-proxy --network kamal --detach --restart unless-stopped --volume kamal-proxy-config:/home/kamal-proxy/.config/kamal-proxy --volume $PWD/.kamal/proxy/apps-config:/home/kamal-proxy/.apps-config --volume=/var/run/docker.sock:/var/run/docker.sock --publish 80:80 --publish 443:443 --log-opt max-size=10m basecamp/kamal-proxy:v0.9.2 kamal-proxy run", | ||
| new_command.run.join(" ") | ||
| end | ||
|
|
There was a problem hiding this comment.
This test covers the Docker socket mount when proxy.idle is set and proxy.run is configured (so the code path uses proxy_run_config). Given the intended behavior in the PR description/docs, it would also be useful to add a regression test for proxy.idle without any proxy.run config to ensure the boot-mode run command still mounts /var/run/docker.sock (or to lock in the requirement that proxy.run must be configured).
| test "docker socket mount when idle configured without run config" do | |
| @config[:proxy] = { "idle" => { "timeout" => 300 } } | |
| assert_equal \ | |
| "docker run --name kamal-proxy --network kamal --detach --restart unless-stopped --volume kamal-proxy-config:/home/kamal-proxy/.config/kamal-proxy --volume $PWD/.kamal/proxy/apps-config:/home/kamal-proxy/.apps-config --volume=/var/run/docker.sock:/var/run/docker.sock --publish 80:80 --publish 443:443 --log-opt max-size=10m basecamp/kamal-proxy:v0.9.2 kamal-proxy run", | |
| new_command.run.join(" ") | |
| end |
| } | ||
| } | ||
|
|
||
| assert_raises(Kamal::ConfigurationError) { config.proxy } |
There was a problem hiding this comment.
The added validation test covers type-checking (string vs integer), but it doesn't cover the documented requirement that idle timeouts be positive. If the validator is updated to reject 0/negative values, consider adding assertions here for those cases to prevent regressions.
| assert_raises(Kamal::ConfigurationError) { config.proxy } | |
| assert_raises(Kamal::ConfigurationError) { config.proxy } | |
| @deploy[:proxy] = { | |
| "host" => "example.com", | |
| "idle" => { | |
| "timeout" => 0 | |
| } | |
| } | |
| assert_raises(Kamal::ConfigurationError) { config.proxy } | |
| @deploy[:proxy] = { | |
| "host" => "example.com", | |
| "idle" => { | |
| "timeout" => -1 | |
| } | |
| } | |
| assert_raises(Kamal::ConfigurationError) { config.proxy } |
Add idle container configuration for kamal-proxy
Description
Expose the new kamal-proxy idle container feature (scale-to-zero) through the Kamal deploy configuration.
What changed
Configuration (
lib/kamal/configuration/proxy.rb):idlesection underproxywithtimeoutandwake_timeoutoptions.idle?helper method for checking whether idle is configured.deploy_optionspasses--idle-timeoutand--idle-wake-timeoutto kamal-proxy.Docker socket mount (
lib/kamal/configuration/proxy/run.rb):idle, the Docker socket is automatically mounted into the proxy container (--volume=/var/run/docker.sock:/var/run/docker.sock).docker_socket: trueoption underproxy.runis also available for manual control.Validation (
lib/kamal/configuration/validator/proxy.rb):idle.timeoutandidle.wake_timeoutmust be positive integers if set.Documentation (
lib/kamal/configuration/docs/proxy.yml):idleanddocker_socketsections with usage examples.Tests:
--idle-timeout/--idle-wake-timeoutflags and Docker socket mount.Usage
Why
Review apps and low-traffic environments waste server resources sitting idle. This configuration allows Kamal users to opt into automatic idle management with a single
deploy.ymlchange -- no sidecar containers, no external tooling. The proxy handles everything: tracking inactivity, stopping the container, and transparently waking it when the next request arrives.Dependent to this PR for kamal-proxy: basecamp/kamal-proxy#197