Merge branch 'main' into pr/99

* main:
  Wording
  Remove accessory images using tags rather than labels
  Update readme to point to ghcr.io/mrsked/mrsk
  Validate that all roles have hosts
  Commander needn't accumulate configuration
  Pull latest image tag, so we can identity it
  Default to deploying the config version
  Remove unneeded Dockerfile.dind, update Readme
  add D-in-D dockerfile, update Readme

README.md

@@ -8,7 +8,19 @@ Join us on Discord: https://discord.gg/YgHVT7GCXS
 
 ## Installation
 
-Install MRSK globally with `gem install mrsk`. Then, inside your app directory, run `mrsk init` (or `mrsk init --bundle` within Rails apps where you want a bin/mrsk binstub). Now edit the new file `config/deploy.yml`. It could look as simple as this:
+If you have a Ruby environment available, you can install MRSK globally with:
+
+```sh
+gem install mrsk
+```
+
+...otherwise, you can run a dockerized version via an alias (add this to your ${SHELL}rc to simplify re-use):
+
+```sh
+alias mrsk='docker run --rm -it -v $HOME/.ssh:/root/.ssh -v /var/run/docker.sock:/var/run/docker.sock -v ${PWD}/:/workdir  ghcr.io/mrsked/mrsk'
+```
+
+Then, inside your app directory, run `mrsk init` (or `mrsk init --bundle` within Rails apps where you want a bin/mrsk binstub). Now edit the new file `config/deploy.yml`. It could look as simple as this:
 
 ```yaml
 service: hey
@@ -36,7 +48,7 @@ mrsk deploy
 This will:
 
 1. Connect to the servers over SSH (using root by default, authenticated by your ssh key)
-2. Install Docker on any server that might be missing it (using apt-get)
+2. Install Docker on any server that might be missing it (using apt-get): root access is needed via ssh for this.
 3. Log into the registry both locally and remotely
 4. Build the image using the standard Dockerfile in the root of the application.
 5. Push the image to the registry.

lib/mrsk/cli/app.rb

@@ -2,7 +2,7 @@ class Mrsk::Cli::App < Mrsk::Cli::Base
   desc "boot", "Boot app on servers (or reboot app if already running)"
   def boot
     say "Get most recent version available as an image...", :magenta unless options[:version]
-    using_version(options[:version] || most_recent_version_available) do |version|
+    using_version(version_or_latest) do |version|
       say "Start container with version #{version} using a #{MRSK.config.readiness_delay}s readiness delay (or reboot if already running)...", :magenta
 
       cli = self
@@ -42,7 +42,7 @@ class Mrsk::Cli::App < Mrsk::Cli::Base
       roles = MRSK.roles_on(host)
 
       roles.each do |role|
-        execute *MRSK.auditor(role: role).record("Started app version #{MRSK.version}"), verbosity: :debug
+        execute *MRSK.auditor.record("Started app version #{MRSK.config.version}"), verbosity: :debug
         execute *MRSK.app(role: role).start, raise_on_non_zero_exit: false
       end
     end
@@ -86,7 +86,7 @@ class Mrsk::Cli::App < Mrsk::Cli::Base
 
     when options[:interactive]
       say "Get most recent version available as an image...", :magenta unless options[:version]
-      using_version(options[:version] || most_recent_version_available) do |version|
+      using_version(version_or_latest) do |version|
         say "Launching interactive command with version #{version} via SSH from new container on #{MRSK.primary_host}...", :magenta
         run_locally { exec MRSK.app.execute_in_new_container_over_ssh(cmd, host: MRSK.primary_host) }
       end
@@ -108,7 +108,7 @@ class Mrsk::Cli::App < Mrsk::Cli::Base
 
     else
       say "Get most recent version available as an image...", :magenta unless options[:version]
-      using_version(options[:version] || most_recent_version_available) do |version|
+      using_version(version_or_latest) do |version|
         say "Launching command with version #{version} from new container...", :magenta
         on(MRSK.hosts) do |host|
           execute *MRSK.auditor.record("Executed cmd '#{cmd}' on app version #{version}"), verbosity: :debug
@@ -217,20 +217,13 @@ class Mrsk::Cli::App < Mrsk::Cli::Base
       end
     end
 
-    def most_recent_version_available(host: MRSK.primary_host)
-      version = nil
-      on(host) { version = capture_with_info(*MRSK.app.most_recent_version_from_available_images).strip }
-      
-      if version == "<none>"
-        raise "Most recent image available was not tagged with a version (returned <none>)"
-      else
-        version.presence
-      end
-    end
-
     def current_running_version(host: MRSK.primary_host)
       version = nil
       on(host) { version = capture_with_info(*MRSK.app.current_running_version).strip }
       version.presence
     end
+
+    def version_or_latest
+      options[:version] || "latest"
+    end
 end

lib/mrsk/cli/base.rb

@@ -39,14 +39,6 @@ module Mrsk::Cli
 
       def initialize_commander(options)
         MRSK.tap do |commander|
-          commander.config_file = Pathname.new(File.expand_path(options[:config_file]))
-          commander.destination = options[:destination]
-          commander.version     = options[:version]
-
-          commander.specific_hosts    = options[:hosts]&.split(",")
-          commander.specific_roles    = options[:roles]&.split(",")
-          commander.specific_primary! if options[:primary]
-
           if options[:verbose]
             ENV["VERBOSE"] = "1" # For backtraces via cli/start
             commander.verbosity = :debug
@@ -55,6 +47,15 @@ module Mrsk::Cli
           if options[:quiet]
             commander.verbosity = :error
           end
+
+          commander.configure \
+            config_file: Pathname.new(File.expand_path(options[:config_file])),
+            destination: options[:destination],
+            version: options[:version]
+
+          commander.specific_hosts    = options[:hosts]&.split(",")
+          commander.specific_roles    = options[:roles]&.split(",")
+          commander.specific_primary! if options[:primary]
         end
       end
 

lib/mrsk/cli/build.rb

@@ -29,7 +29,7 @@ class Mrsk::Cli::Build < Mrsk::Cli::Base
   desc "pull", "Pull app image from registry onto servers"
   def pull
     on(MRSK.hosts) do
-      execute *MRSK.auditor.record("Pulled image with version #{MRSK.version}"), verbosity: :debug
+      execute *MRSK.auditor.record("Pulled image with version #{MRSK.config.version}"), verbosity: :debug
       execute *MRSK.builder.clean, raise_on_non_zero_exit: false
       execute *MRSK.builder.pull
     end

lib/mrsk/cli/main.rb

@@ -11,7 +11,7 @@ class Mrsk::Cli::Main < Mrsk::Cli::Base
   desc "deploy", "Deploy app to servers"
   option :skip_push, aliases: "-P", type: :boolean, default: false, desc: "Skip image build and push"
   def deploy
-    invoke_options = options.without(:skip_push)
+    invoke_options = deploy_options
 
     runtime = print_runtime do
       say "Ensure curl and Docker are installed...", :magenta
@@ -46,7 +46,7 @@ class Mrsk::Cli::Main < Mrsk::Cli::Base
   desc "redeploy", "Deploy app to servers without bootstrapping servers, starting Traefik, pruning, and registry login"
   option :skip_push, aliases: "-P", type: :boolean, default: false, desc: "Skip image build and push"
   def redeploy
-    invoke_options = options.without(:skip_push)
+    invoke_options = deploy_options
 
     runtime = print_runtime do
       if options[:skip_push]
@@ -68,7 +68,7 @@ class Mrsk::Cli::Main < Mrsk::Cli::Base
 
   desc "rollback [VERSION]", "Rollback app to VERSION"
   def rollback(version)
-    MRSK.version = version
+    MRSK.config.version = version
 
     if container_name_available?(MRSK.config.service_with_version)
       say "Start version #{version}, then wait #{MRSK.config.readiness_delay}s for app to boot before stopping the old version...", :magenta
@@ -207,6 +207,10 @@ class Mrsk::Cli::Main < Mrsk::Cli::Base
       Array(container_names).include?(container_name)
     end
 
+    def deploy_options
+      { "version" => MRSK.config.version }.merge(options.without("skip_push"))
+    end
+
     def service_version(version = MRSK.config.abbreviated_version)
       [ MRSK.config.service, version ].compact.join("@")
     end

lib/mrsk/commander.rb

@@ -1,17 +1,22 @@
 require "active_support/core_ext/enumerable"
+require "active_support/core_ext/module/delegation"
 
 class Mrsk::Commander
-  attr_accessor :config_file, :destination, :verbosity, :version
+  attr_accessor :verbosity
 
-  def initialize(config_file: nil, destination: nil, verbosity: :info)
+  def initialize
-    @config_file, @destination, @verbosity = config_file, destination, verbosity
+    self.verbosity = :info
   end
 
   def config
-    @config ||= \
+    @config ||= Mrsk::Configuration.create_from(**@config_kwargs).tap do |config|
-      Mrsk::Configuration
+      @config_kwargs = nil
-        .create_from(config_file, destination: destination, version: cascading_version)
+      configure_sshkit_with(config)
-        .tap { |config| configure_sshkit_with(config) }
+    end
+  end
+
+  def configure(**kwargs)
+    @config, @config_kwargs = nil, kwargs
   end
 
   attr_reader :specific_roles, :specific_hosts
@@ -106,26 +111,15 @@ class Mrsk::Commander
     SSHKit.config.output_verbosity = old_level
   end
 
+
   # Test-induced damage!
   def reset
-    @config = @config_file = @destination = @version = nil
+    @config = nil
     @app = @builder = @traefik = @registry = @prune = @auditor = nil
     @verbosity = :info
   end
 
   private
-    def cascading_version
-      version.presence || ENV["VERSION"] || current_commit_hash
-    end
-
-    def current_commit_hash
-      if system("git rev-parse")
-        `git rev-parse HEAD`.strip
-      else
-        raise "Can't use commit hash as version, no git repository found in #{Dir.pwd}"
-      end
-    end
-
     # Lazy setup of SSHKit
     def configure_sshkit_with(config)
       SSHKit::Backend::Netssh.configure { |ssh| ssh.ssh_options = config.ssh_options }

lib/mrsk/commands/accessory.rb

@@ -100,7 +100,7 @@ class Mrsk::Commands::Accessory < Mrsk::Commands::Base
   end
 
   def remove_image
-    docker :image, :prune, "--all", "--force", *service_filter
+    docker :image, :rm, "--force", image
   end
 
   private

lib/mrsk/commands/app.rb

@@ -94,19 +94,6 @@ class Mrsk::Commands::App < Mrsk::Commands::Base
       "tail -n 1"
   end
 
-  def most_recent_version_from_available_images
-    pipe \
-      docker(:image, :ls, "--format", '"{{.Tag}}"', config.repository),
-      "head -n 1"
-  end
-
-  def all_versions_from_available_containers
-    pipe \
-      docker(:image, :ls, "--format", '"{{.Tag}}"', config.repository),
-      "head -n 1"
-  end
-
-
   def list_containers
     docker :container, :ls, "--all", *filter_args
   end

lib/mrsk/commands/builder/base.rb

@@ -7,6 +7,7 @@ class Mrsk::Commands::Builder::Base < Mrsk::Commands::Base
 
   def pull
     docker :pull, config.absolute_image
+    docker :pull, config.latest_image
   end
 
   def build_options

lib/mrsk/configuration.rb

@@ -9,13 +9,12 @@ class Mrsk::Configuration
   delegate :service, :image, :servers, :env, :labels, :registry, :builder, to: :raw_config, allow_nil: true
   delegate :argumentize, :argumentize_env_with_secrets, to: Mrsk::Utils
 
-  attr_accessor :version
   attr_accessor :destination
   attr_accessor :raw_config
 
   class << self
-    def create_from(base_config_file, destination: nil, version: "missing")
+    def create_from(config_file:, destination: nil, version: nil)
-      raw_config = load_config_files(base_config_file, *destination_config_file(base_config_file, destination))
+      raw_config = load_config_files(config_file, *destination_config_file(config_file, destination))
 
       new raw_config, destination: destination, version: version
     end
@@ -38,14 +37,22 @@ class Mrsk::Configuration
       end
   end
 
-  def initialize(raw_config, destination: nil, version: "missing", validate: true)
+  def initialize(raw_config, destination: nil, version: nil, validate: true)
     @raw_config = ActiveSupport::InheritableOptions.new(raw_config)
     @destination = destination
-    @version = version
+    @declared_version = version
     valid? if validate
   end
 
 
+  def version=(version)
+    @declared_version = version
+  end
+
+  def version
+    @declared_version.presence || ENV["VERSION"] || current_commit_hash
+  end
+
   def abbreviated_version
     Mrsk::Utils.abbreviate_version(version)
   end
@@ -73,7 +80,7 @@ class Mrsk::Configuration
   end
 
   def primary_web_host
-    role(:web).hosts.first
+    role(:web).primary_host
   end
 
   def traefik_hosts
@@ -189,6 +196,12 @@ class Mrsk::Configuration
         raise ArgumentError, "You must specify a password for the registry in config/deploy.yml (or set the ENV variable if that's used)"
       end
 
+      roles.each do |role|
+        if role.hosts.empty?
+          raise ArgumentError, "No servers specified for the #{role.name} role"
+        end
+      end
+
       true
     end
 
@@ -203,4 +216,13 @@ class Mrsk::Configuration
     def role_names
       raw_config.servers.is_a?(Array) ? [ "web" ] : raw_config.servers.keys.sort
     end
+
+    def current_commit_hash
+      @current_commit_hash ||=
+        if system("git rev-parse")
+          `git rev-parse HEAD`.strip
+        else
+          raise "Can't use commit hash as version, no git repository found in #{Dir.pwd}"
+        end
+    end
 end

lib/mrsk/configuration/role.rb

@@ -7,6 +7,10 @@ class Mrsk::Configuration::Role
    @name, @config = name.inquiry, config
   end
 
+  def primary_host
+    hosts.first
+  end
+
   def hosts
     @hosts ||= extract_hosts_from_config
   end
@@ -55,7 +59,7 @@ class Mrsk::Configuration::Role
         config.servers
       else
         servers = config.servers[name]
-        servers.is_a?(Array) ? servers : servers["hosts"]
+        servers.is_a?(Array) ? servers : Array(servers["hosts"])
       end
     end
 

test/cli/accessory_test.rb

@@ -126,7 +126,7 @@ class CliAccessoryTest < CliTestCase
   end
 
   test "remove_image" do
-    assert_match "docker image prune --all --force --filter label=service=app-mysql", run_command("remove_image", "mysql")
+    assert_match "docker image rm --force mysql", run_command("remove_image", "mysql")
   end
 
   test "remove_service_directory" do

test/cli/app_test.rb

@@ -26,9 +26,8 @@ class CliAppTest < CliTestCase
       .returns([ :docker, :run ])
 
     run_command("boot").tap do |output|
-      assert_match "Rebooting container with same version 999 already deployed", output # Can't start what's already running
+      assert_match "Rebooting container with same version latest already deployed", output # Can't start what's already running
-      assert_match "docker container ls --all --filter name=app-web-999 --quiet | xargs docker container rm", output # Stop old running
+      assert_match "docker container ls --all --filter name=app-latest --quiet | xargs docker container rm", output # Remove old container
-      assert_match "docker container ls --all --filter name=app-web-999 --quiet | xargs docker container rm", output # Remove old container
       assert_match "docker run", output # Start new container
     end
   ensure

test/cli/build_test.rb

@@ -29,7 +29,7 @@ class CliBuildTest < CliTestCase
   test "pull" do
     run_command("pull").tap do |output|
       assert_match /docker image rm --force dhh\/app:999/, output
-      assert_match /docker pull dhh\/app:999/, output
+      assert_match /docker pull dhh\/app:latest/, output
     end
   end
 

test/cli/main_test.rb

@@ -10,7 +10,7 @@ class CliMainTest < CliTestCase
   end
 
   test "deploy" do
-    invoke_options = { "config_file" => "test/fixtures/deploy_with_accessories.yml", "skip_broadcast" => false, "skip_push" => false }
+    invoke_options = { "config_file" => "test/fixtures/deploy_with_accessories.yml", "skip_broadcast" => false, "version" => "999" }
 
     Mrsk::Cli::Main.any_instance.expects(:invoke).with("mrsk:cli:server:bootstrap", [], invoke_options)
     Mrsk::Cli::Main.any_instance.expects(:invoke).with("mrsk:cli:registry:login", [], invoke_options)
@@ -31,7 +31,7 @@ class CliMainTest < CliTestCase
   end
 
   test "deploy with skip_push" do
-    invoke_options = { "config_file" => "test/fixtures/deploy_with_accessories.yml", "skip_broadcast" => false, "skip_push" => true }
+    invoke_options = { "config_file" => "test/fixtures/deploy_with_accessories.yml", "skip_broadcast" => false, "version" => "999" }
 
     Mrsk::Cli::Main.any_instance.expects(:invoke).with("mrsk:cli:server:bootstrap", [], invoke_options)
     Mrsk::Cli::Main.any_instance.expects(:invoke).with("mrsk:cli:registry:login", [], invoke_options)
@@ -52,7 +52,7 @@ class CliMainTest < CliTestCase
   end
 
   test "redeploy" do
-    invoke_options = { "config_file" => "test/fixtures/deploy_with_accessories.yml", "skip_broadcast" => false, "skip_push" => false}
+    invoke_options = { "config_file" => "test/fixtures/deploy_with_accessories.yml", "skip_broadcast" => false, "version" => "999" }
 
     Mrsk::Cli::Main.any_instance.expects(:invoke).with("mrsk:cli:build:deliver", [], invoke_options)
     Mrsk::Cli::Main.any_instance.expects(:invoke).with("mrsk:cli:healthcheck:perform", [], invoke_options)
@@ -65,7 +65,7 @@ class CliMainTest < CliTestCase
   end
 
   test "redeploy with skip_push" do
-    invoke_options = { "config_file" => "test/fixtures/deploy_with_accessories.yml", "skip_broadcast" => false, "skip_push" => true }
+    invoke_options = { "config_file" => "test/fixtures/deploy_with_accessories.yml", "skip_broadcast" => false, "version" => "999" }
 
     Mrsk::Cli::Main.any_instance.expects(:invoke).with("mrsk:cli:build:pull", [], invoke_options)
     Mrsk::Cli::Main.any_instance.expects(:invoke).with("mrsk:cli:healthcheck:perform", [], invoke_options)
@@ -124,7 +124,7 @@ class CliMainTest < CliTestCase
   end
 
   test "config" do
-    run_command("config").tap do |output|
+    run_command("config", config_file: "deploy_with_accessories").tap do |output|
       config = YAML.load(output)
 
       assert_equal ["web"], config[:roles]
@@ -136,6 +136,32 @@ class CliMainTest < CliTestCase
     end
   end
 
+  test "config with roles" do
+    run_command("config", config_file: "deploy_with_roles").tap do |output|
+      config = YAML.load(output)
+
+      assert_equal ["web", "workers"], config[:roles]
+      assert_equal ["1.1.1.1", "1.1.1.2", "1.1.1.3", "1.1.1.4"], config[:hosts]
+      assert_equal "999", config[:version]
+      assert_equal "registry.digitalocean.com/dhh/app", config[:repository]
+      assert_equal "registry.digitalocean.com/dhh/app:999", config[:absolute_image]
+      assert_equal "app-999", config[:service_with_version]
+    end
+  end
+
+  test "config with destination" do
+    run_command("config", "-d", "world", config_file: "deploy_for_dest").tap do |output|
+      config = YAML.load(output)
+
+      assert_equal ["web"], config[:roles]
+      assert_equal ["1.1.1.1", "1.1.1.2"], config[:hosts]
+      assert_equal "999", config[:version]
+      assert_equal "registry.digitalocean.com/dhh/app", config[:repository]
+      assert_equal "registry.digitalocean.com/dhh/app:999", config[:absolute_image]
+      assert_equal "app-999", config[:service_with_version]
+    end
+  end
+
   test "init" do
     Pathname.any_instance.expects(:exist?).returns(false).twice
     FileUtils.stubs(:mkdir_p)
@@ -207,12 +233,12 @@ class CliMainTest < CliTestCase
 
       assert_match /docker container stop app-mysql/, output
       assert_match /docker container prune --force --filter label=service=app-mysql/, output
-      assert_match /docker image prune --all --force --filter label=service=app-mysql/, output
+      assert_match /docker image rm --force mysql/, output
       assert_match /rm -rf app-mysql/, output
 
       assert_match /docker container stop app-redis/, output
       assert_match /docker container prune --force --filter label=service=app-redis/, output
-      assert_match /docker image prune --all --force --filter label=service=app-redis/, output
+      assert_match /docker image rm --force redis/, output
       assert_match /rm -rf app-redis/, output
 
       assert_match /docker logout/, output
@@ -225,7 +251,7 @@ class CliMainTest < CliTestCase
   end
 
   private
-    def run_command(*command)
+    def run_command(*command, config_file: "deploy_with_accessories")
-      stdouted { Mrsk::Cli::Main.start([*command, "-c", "test/fixtures/deploy_with_accessories.yml"]) }
+      stdouted { Mrsk::Cli::Main.start([*command, "-c", "test/fixtures/#{config_file}.yml"]) }
     end
 end

test/commander_test.rb

@@ -2,7 +2,9 @@ require "test_helper"
 
 class CommanderTest < ActiveSupport::TestCase
   setup do
-    @mrsk = Mrsk::Commander.new config_file: Pathname.new(File.expand_path("fixtures/deploy_with_roles.yml", __dir__))
+    @mrsk = Mrsk::Commander.new.tap do |mrsk|
+      mrsk.configure config_file: Pathname.new(File.expand_path("fixtures/deploy_with_roles.yml", __dir__))
+    end
   end
 
   test "lazy configuration" do
@@ -19,8 +21,8 @@ class CommanderTest < ActiveSupport::TestCase
     assert_match /no git repository found/, error.message
   end
 
-  test "filtering hosts" do
+  test "overwriting hosts" do
-    assert_equal [ "1.1.1.1", "1.1.1.2", "1.1.1.3" ], @mrsk.hosts
+    assert_equal [ "1.1.1.1", "1.1.1.2", "1.1.1.3", "1.1.1.4" ], @mrsk.hosts
 
     @mrsk.specific_hosts = [ "1.1.1.1", "1.1.1.2" ]
     assert_equal [ "1.1.1.1", "1.1.1.2" ], @mrsk.hosts

test/commands/accessory_test.rb

@@ -137,7 +137,7 @@ class CommandsAccessoryTest < ActiveSupport::TestCase
 
   test "remove image" do
     assert_equal \
-      "docker image prune --all --force --filter label=service=app-mysql",
+      "docker image rm --force private.registry/mysql:8.0",
       @mysql.remove_image.join(" ")
   end
 end

test/commands/app_test.rb

@@ -188,12 +188,6 @@ class CommandsAppTest < ActiveSupport::TestCase
       new_command.current_running_version.join(" ")
   end
 
-  test "most_recent_version_from_available_images" do
-    assert_equal \
-      "docker image ls --format \"{{.Tag}}\" dhh/app | head -n 1",
-      new_command.most_recent_version_from_available_images.join(" ")
-  end
-
   test "list_containers" do
     assert_equal \
       "docker container ls --all --filter label=service=app --filter label=role=web",

test/configuration_test.rb

@@ -3,6 +3,7 @@ require "test_helper"
 class ConfigurationTest < ActiveSupport::TestCase
   setup do
     ENV["RAILS_MASTER_KEY"] = "456"
+    ENV["VERSION"] = "missing"
 
     @deploy = {
       service: "app", image: "dhh/app",
@@ -21,17 +22,23 @@ class ConfigurationTest < ActiveSupport::TestCase
   end
 
   teardown do
-    ENV["RAILS_MASTER_KEY"] = nil
+    ENV.delete("RAILS_MASTER_KEY")
+    ENV.delete("VERSION")
   end
 
-  test "ensure valid keys" do
+  %i[ service image registry ].each do |key|
-    assert_raise(ArgumentError) do
+    test "#{key} config required" do
-      Mrsk::Configuration.new(@deploy.tap { _1.delete(:service) })
+      assert_raise(ArgumentError) do
-      Mrsk::Configuration.new(@deploy.tap { _1.delete(:image) })
+        Mrsk::Configuration.new @deploy.tap { _1.delete key }
-      Mrsk::Configuration.new(@deploy.tap { _1.delete(:registry) })
+      end
+    end
+  end
 
-      Mrsk::Configuration.new(@deploy.tap { _1[:registry].delete("username") })
+  %w[ username password ].each do |key|
-      Mrsk::Configuration.new(@deploy.tap { _1[:registry].delete("password") })
+    test "registry #{key} required" do
+      assert_raise(ArgumentError) do
+        Mrsk::Configuration.new @deploy.tap { _1[:registry].delete key }
+      end
     end
   end
 
@@ -66,8 +73,20 @@ class ConfigurationTest < ActiveSupport::TestCase
   end
 
   test "version" do
-    assert_equal "missing", @config.version
+    ENV.delete("VERSION")
-    assert_equal "123", Mrsk::Configuration.new(@deploy, version: "123").version
+
+    @config.expects(:system).with("git rev-parse").returns(nil)
+    error = assert_raises(RuntimeError) { @config.version}
+    assert_match /no git repository found/, error.message
+
+    @config.expects(:current_commit_hash).returns("git-version")
+    assert_equal "git-version", @config.version
+
+    ENV["VERSION"] = "env-version"
+    assert_equal "env-version", @config.version
+
+    @config.version = "arg-version"
+    assert_equal "arg-version", @config.version
   end
 
   test "repository" do
@@ -134,6 +153,39 @@ class ConfigurationTest < ActiveSupport::TestCase
 
   test "valid config" do
     assert @config.valid?
+    assert @config_with_roles.valid?
+  end
+
+  test "hosts required for all roles" do
+    # Empty server list for implied web role
+    assert_raises(ArgumentError) do
+      Mrsk::Configuration.new @deploy.merge(servers: [])
+    end
+
+    # Empty server list
+    assert_raises(ArgumentError) do
+      Mrsk::Configuration.new @deploy.merge(servers: { "web" => [] })
+    end
+
+    # Missing hosts key
+    assert_raises(ArgumentError) do
+      Mrsk::Configuration.new @deploy.merge(servers: { "web" => {} })
+    end
+
+    # Empty hosts list
+    assert_raises(ArgumentError) do
+      Mrsk::Configuration.new @deploy.merge(servers: { "web" => { "hosts" => [] } })
+    end
+
+    # Nil hosts
+    assert_raises(ArgumentError) do
+      Mrsk::Configuration.new @deploy.merge(servers: { "web" => { "hosts" => nil } })
+    end
+
+    # One role with hosts, one without
+    assert_raises(ArgumentError) do
+      Mrsk::Configuration.new @deploy.merge(servers: { "web" => %w[ web ], "workers" => { "hosts" => %w[ ] } })
+    end
   end
 
   test "ssh options" do
@@ -158,17 +210,17 @@ class ConfigurationTest < ActiveSupport::TestCase
   end
 
   test "erb evaluation of yml config" do
-    config = Mrsk::Configuration.create_from Pathname.new(File.expand_path("fixtures/deploy.erb.yml", __dir__))
+    config = Mrsk::Configuration.create_from config_file: Pathname.new(File.expand_path("fixtures/deploy.erb.yml", __dir__))
     assert_equal "my-user", config.registry["username"]
   end
 
   test "destination yml config merge" do
     dest_config_file = Pathname.new(File.expand_path("fixtures/deploy_for_dest.yml", __dir__))
 
-    config = Mrsk::Configuration.create_from dest_config_file, destination: "world"
+    config = Mrsk::Configuration.create_from config_file: dest_config_file, destination: "world"
     assert_equal "1.1.1.1", config.all_hosts.first
 
-    config = Mrsk::Configuration.create_from dest_config_file, destination: "mars"
+    config = Mrsk::Configuration.create_from config_file: dest_config_file, destination: "mars"
     assert_equal "1.1.1.3", config.all_hosts.first
   end
 
@@ -176,7 +228,7 @@ class ConfigurationTest < ActiveSupport::TestCase
     dest_config_file = Pathname.new(File.expand_path("fixtures/deploy_for_dest.yml", __dir__))
 
     assert_raises(RuntimeError) do
-      config = Mrsk::Configuration.create_from dest_config_file, destination: "missing"
+      config = Mrsk::Configuration.create_from config_file: dest_config_file, destination: "missing"
     end
   end
 

test/fixtures/deploy_with_roles.yml

@@ -5,8 +5,9 @@ servers:
     - 1.1.1.1
     - 1.1.1.2
   workers:
-    - 1.1.1.1
+    hosts:
-    - 1.1.1.3
+      - 1.1.1.3
+      - 1.1.1.4
 env:
   REDIS_URL: redis://x/y
 registry: