Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow Kamal to run without traefik #580

Merged
merged 3 commits into from
Nov 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/kamal/cli/healthcheck.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ class Kamal::Cli::Healthcheck < Kamal::Cli::Base

desc "perform", "Health check current app version"
def perform
raise "The primary host is not configured to run Traefik" unless KAMAL.config.role(KAMAL.config.primary_role).running_traefik?
Copy link
Contributor Author

@yoelcabo yoelcabo Nov 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

KAMAL.config.role(KAMAL.config.primary_role) might be too verbose.

In my original changes, KAMAL.config.primary_role was an instance of Role.

However, with #577, KAMAL.config.primary_web_role was introduced as a String.

I didn't want to change that to make the review simpler and focusing on what matters. However, if you think a cleanup might be interesting, I can submit another PR afterwards

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @yoelcabo - yes if you could raise another PR to convert KAMAL.config.primary_role to a Role that would be great. I'll merge this PR now 🙏

on(KAMAL.primary_host) do
begin
execute *KAMAL.healthcheck.run
Expand Down
6 changes: 4 additions & 2 deletions lib/kamal/cli/main.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,10 @@ def deploy
say "Ensure Traefik is running...", :magenta
invoke "kamal:cli:traefik:boot", [], invoke_options

say "Ensure app can pass healthcheck...", :magenta
invoke "kamal:cli:healthcheck:perform", [], invoke_options
if KAMAL.config.role(KAMAL.config.primary_role).running_traefik?
say "Ensure app can pass healthcheck...", :magenta
invoke "kamal:cli:healthcheck:perform", [], invoke_options
end

say "Detect stale containers...", :magenta
invoke "kamal:cli:app:stale_containers", [], invoke_options.merge(stop: true)
Expand Down
5 changes: 2 additions & 3 deletions lib/kamal/cli/templates/deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,12 @@ registry:
# limit: 10 # Can also specify as a percentage of total hosts, such as "25%"
# wait: 2

# Configure the role used to determine the primary_web_host. This host takes
# Configure the role used to determine the primary_host. This host takes
# deploy locks, runs health checks during the deploy, and follow logs, etc.
# This role should have traefik enabled.
#
# Caution: there's no support for role renaming yet, so be careful to cleanup
# the previous role on the deployed hosts.
# primary_web_role: web
# primary_role: web

# Controls if we abort when see a role with no hosts. Disabling this may be
# useful for more complex deploy configurations.
Expand Down
4 changes: 2 additions & 2 deletions lib/kamal/commander.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def configure(**kwargs)
attr_reader :specific_roles, :specific_hosts

def specific_primary!
self.specific_hosts = [ config.primary_web_host ]
self.specific_hosts = [ config.primary_host ]
end

def specific_roles=(role_names)
Expand All @@ -36,7 +36,7 @@ def specific_hosts=(hosts)
end

def primary_host
specific_hosts&.first || specific_roles&.first&.primary_host || config.primary_web_host
specific_hosts&.first || specific_roles&.first&.primary_host || config.primary_host
end

def primary_role
Expand Down
10 changes: 5 additions & 5 deletions lib/kamal/commands/healthcheck.rb
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
class Kamal::Commands::Healthcheck < Kamal::Commands::Base

def run
web = config.role(config.primary_web_role)
primary = config.role(config.primary_role)

docker :run,
"--detach",
"--name", container_name_with_version,
"--publish", "#{exposed_port}:#{config.healthcheck["port"]}",
"--label", "service=#{config.healthcheck_service}",
"-e", "KAMAL_CONTAINER_NAME=\"#{config.healthcheck_service}\"",
*web.env_args,
*web.health_check_args(cord: false),
*primary.env_args,
*primary.health_check_args(cord: false),
*config.volume_args,
*web.option_args,
*primary.option_args,
config.absolute_image,
web.cmd
primary.cmd
end

def status
Expand Down
22 changes: 9 additions & 13 deletions lib/kamal/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@ def all_hosts
roles.flat_map(&:hosts).uniq
end

def primary_web_host
role(primary_web_role)&.primary_host
def primary_host
role(primary_role)&.primary_host
end

def traefik_roles
Expand Down Expand Up @@ -208,8 +208,8 @@ def asset_path
raw_config.asset_path
end

def primary_web_role
raw_config.primary_web_role || "web"
def primary_role
raw_config.primary_role || "web"
end

def allow_empty_roles?
Expand All @@ -225,7 +225,7 @@ def to_h
{
roles: role_names,
hosts: all_hosts,
primary_host: primary_web_host,
primary_host: primary_host,
version: version,
repository: repository,
absolute_image: absolute_image,
Expand Down Expand Up @@ -264,16 +264,12 @@ def ensure_required_keys_present
raise ArgumentError, "You must specify a password for the registry in config/deploy.yml (or set the ENV variable if that's used)"
end

unless role_names.include?(primary_web_role)
raise ArgumentError, "The primary_web_role #{primary_web_role} isn't defined"
unless role_names.include?(primary_role)
raise ArgumentError, "The primary_role #{primary_role} isn't defined"
end

unless traefik_role_names.include?(primary_web_role)
raise ArgumentError, "Role #{primary_web_role} needs to have traefik enabled"
end

if role(primary_web_role).hosts.empty?
raise ArgumentError, "No servers specified for the #{primary_web_role} primary_web_role"
if role(primary_role).hosts.empty?
raise ArgumentError, "No servers specified for the #{primary_role} primary_role"
end

unless allow_empty_roles?
Expand Down
10 changes: 9 additions & 1 deletion lib/kamal/configuration/role.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,15 @@ def health_check_interval


def running_traefik?
name.web? || specializations["traefik"]
if specializations["traefik"].nil?
primary?
else
specializations["traefik"]
end
end

def primary?
@config.primary_role == name
end


Expand Down
14 changes: 12 additions & 2 deletions test/cli/healthcheck_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,19 @@ class CliHealthcheckTest < CliTestCase
end
assert_match "container not ready (unhealthy)", exception.message
end

test "raises an exception if primary does not have traefik" do
SSHKit::Backend::Abstract.any_instance.expects(:execute).never

exception = assert_raises do
run_command("perform", config_file: "test/fixtures/deploy_workers_only.yml")
end

assert_equal "The primary host is not configured to run Traefik", exception.message
end

private
def run_command(*command)
stdouted { Kamal::Cli::Healthcheck.start([*command, "-c", "test/fixtures/deploy_with_accessories.yml"]) }
def run_command(*command, config_file: "test/fixtures/deploy_with_accessories.yml")
stdouted { Kamal::Cli::Healthcheck.start([*command, "-c", config_file]) }
end
end
15 changes: 15 additions & 0 deletions test/cli/main_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,21 @@ class CliMainTest < CliTestCase
refute_match /Running the post-deploy hook.../, output
end
end

test "deploy without healthcheck if primary host doesn't have traefik" do
invoke_options = { "config_file" => "test/fixtures/deploy_workers_only.yml", "version" => "999", "skip_hooks" => false }

Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:healthcheck:perform", [], invoke_options).never

Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:registry:login", [], invoke_options)
Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:build:deliver", [], invoke_options)
Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:traefik:boot", [], invoke_options)
Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:app:stale_containers", [], invoke_options.merge(stop: true))
Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:app:boot", [], invoke_options)
Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:prune:all", [], invoke_options)

run_command("deploy", config_file: "deploy_workers_only")
end

test "deploy with missing secrets" do
invoke_options = { "config_file" => "test/fixtures/deploy_with_secrets.yml", "version" => "999", "skip_hooks" => false }
Expand Down
30 changes: 13 additions & 17 deletions test/configuration_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ class ConfigurationTest < ActiveSupport::TestCase
assert_equal [ "1.1.1.1", "1.1.1.2", "1.1.1.3" ], @config_with_roles.all_hosts
end

test "primary web host" do
assert_equal "1.1.1.1", @config.primary_web_host
assert_equal "1.1.1.1", @config_with_roles.primary_web_host
test "primary host" do
assert_equal "1.1.1.1", @config.primary_host
assert_equal "1.1.1.1", @config_with_roles.primary_host
end

test "traefik hosts" do
Expand Down Expand Up @@ -289,27 +289,23 @@ class ConfigurationTest < ActiveSupport::TestCase
assert_equal "foo", Kamal::Configuration.new(@deploy.merge!(asset_path: "foo")).asset_path
end

test "primary web role" do
assert_equal "web", @config.primary_web_role
test "primary role" do
assert_equal "web", @config.primary_role

config = Kamal::Configuration.new(@deploy_with_roles.deep_merge({
servers: { "alternate_web" => { "hosts" => [ "1.1.1.4", "1.1.1.5" ] , "traefik" => true } },
primary_web_role: "alternate_web" } ))
servers: { "alternate_web" => { "hosts" => [ "1.1.1.4", "1.1.1.5" ] } },
primary_role: "alternate_web" } ))

assert_equal "alternate_web", config.primary_web_role
assert_equal "1.1.1.4", config.primary_web_host
end

test "primary web role no traefik" do
error = assert_raises(ArgumentError) do
Kamal::Configuration.new(@deploy_with_roles.merge(primary_web_role: "workers"))
end
assert_match /workers needs to have traefik enabled/, error.message
assert_equal "alternate_web", config.primary_role
assert_equal "1.1.1.4", config.primary_host
assert config.role(:alternate_web).primary?
assert config.role(:alternate_web).running_traefik?
end

test "primary web role missing" do
test "primary role missing" do
error = assert_raises(ArgumentError) do
Kamal::Configuration.new(@deploy.merge(primary_web_role: "bar"))
Kamal::Configuration.new(@deploy.merge(primary_role: "bar"))
end
assert_match /bar isn't defined/, error.message
end
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/deploy_primary_web_role_override.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,4 @@ registry:
server: registry.digitalocean.com
username: user
password: pw
primary_web_role: web_tokyo
primary_role: web_tokyo
12 changes: 12 additions & 0 deletions test/fixtures/deploy_workers_only.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
service: app
image: dhh/app
servers:
workers:
traefik: false
hosts:
- 1.1.1.1
- 1.1.1.2
primary_role: workers
registry:
username: user
password: pw