From 43a1b42f8c522f70f8dd5e70b68d0fb48ea89949 Mon Sep 17 00:00:00 2001
From: Stephen van Beek <stephen@happyvalley.io>
Date: Fri, 10 Mar 2023 09:39:14 +0000
Subject: [PATCH 01/19] Added the additional_ports configuration

ISSUE: https://github.com/mrsked/mrsk/issues/98
---
 README.md                     | 36 +++++++++++++++++++++++++++++++++++
 lib/mrsk/commands/traefik.rb  |  9 +++++++++
 test/commands/traefik_test.rb | 11 +++++++++++
 3 files changed, 56 insertions(+)

diff --git a/README.md b/README.md
index 56d26116..eb1f0aab 100644
--- a/README.md
+++ b/README.md
@@ -426,6 +426,42 @@ traefik:
   host_port: 8080
 ```
 
+### Additional entrypoints for traefik
+
+You can configure additional ports and entrypoints for traefik list so:
+
+```yaml
+traefik:
+  additional_ports: 
+    - 9000
+    - 9001
+  args:
+    entrypoints.myentrypoint.address: ":9000"
+    entrypoints.otherentrypoint.address: ":9001"
+```
+
+Be aware, a lot of the out-of-the-box magic of mrsk is provided by traefik defaults so going down this path requires more manual configuration, like so:
+
+A more complete example including entrypoints would be:
+
+```yaml
+service: myservice
+
+labels:
+  traefik.tcp.routers.other.rule: 'HostSNI(`*`)'
+  traefik.tcp.routers.other.entrypoints: otherentrypoint
+  traefik.tcp.services.other.loadbalancer.server.port: 9000
+  traefik.http.routers.myservice.entrypoints: web
+  traefik.http.services.myservice.loadbalancer.server.port: 8080
+
+traefik:
+  additional_ports:
+    - 9000
+  args:
+    'entrypoints.web.address=:80': true
+    'entrypoints.otherentrypoint.address=:9000': true
+```
+
 ### Configuring build args for new images
 
 Build arguments that aren't secret can also be configured:
diff --git a/lib/mrsk/commands/traefik.rb b/lib/mrsk/commands/traefik.rb
index b9ff19c1..acda3cf5 100644
--- a/lib/mrsk/commands/traefik.rb
+++ b/lib/mrsk/commands/traefik.rb
@@ -9,6 +9,7 @@ class Mrsk::Commands::Traefik < Mrsk::Commands::Base
       "--restart", "unless-stopped",
       "--log-opt", "max-size=#{MAX_LOG_SIZE}",
       "--publish", port,
+      *additional_ports,
       "--volume", "/var/run/docker.sock:/var/run/docker.sock",
       "traefik",
       "--providers.docker",
@@ -54,6 +55,14 @@ class Mrsk::Commands::Traefik < Mrsk::Commands::Base
   end
 
   private
+    def additional_ports
+      if args = config.raw_config.dig(:traefik, "additional_ports")
+        args.collect { |value| "--publish #{value}:#{value}" }.compact
+      else
+        []
+      end
+    end
+
     def cmd_option_args
       if args = config.traefik["args"]
         optionize args, with: "="
diff --git a/test/commands/traefik_test.rb b/test/commands/traefik_test.rb
index 5ca51e13..58d0a557 100644
--- a/test/commands/traefik_test.rb
+++ b/test/commands/traefik_test.rb
@@ -19,6 +19,17 @@ class CommandsTraefikTest < ActiveSupport::TestCase
       new_command.run.join(" ")
   end
 
+  test "run with additional entrypoints" do
+    assert_equal \
+      "docker run --name traefik --detach --restart unless-stopped --log-opt max-size=10m --publish 80:80 --volume /var/run/docker.sock:/var/run/docker.sock traefik --providers.docker --log.level=DEBUG --accesslog.format \"json\" --metrics.prometheus.buckets \"0.1,0.3,1.2,5.0\"",
+      new_command.run.join(" ")
+
+    @config[:traefik]["additional_ports"] = %w[9000 9001]
+    assert_equal \
+      "docker run --name traefik --detach --restart unless-stopped --log-opt max-size=10m --publish 80:80 --publish 9000:9000 --publish 9001:9001 --volume /var/run/docker.sock:/var/run/docker.sock traefik --providers.docker --log.level=DEBUG --accesslog.format \"json\" --metrics.prometheus.buckets \"0.1,0.3,1.2,5.0\"",
+      new_command.run.join(" ")
+  end
+
   test "run without configuration" do
     @config.delete(:traefik)
 

From 2cea12c56bc0330c9281dd321f69e9040dfe783c Mon Sep 17 00:00:00 2001
From: Stephen van Beek <stephen@happyvalley.io>
Date: Fri, 10 Mar 2023 14:08:06 +0000
Subject: [PATCH 02/19] Modified in response to PR comments

---
 README.md                     | 9 +++++----
 lib/mrsk/commands/traefik.rb  | 9 ++++-----
 test/commands/traefik_test.rb | 6 +++---
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/README.md b/README.md
index eb1f0aab..448d8aaf 100644
--- a/README.md
+++ b/README.md
@@ -426,13 +426,13 @@ traefik:
   host_port: 8080
 ```
 
-### Additional entrypoints for traefik
+### Configure entrypoints for traefik
 
-You can configure additional ports and entrypoints for traefik list so:
+You can override the ports and entrypoints for traefik list so:
 
 ```yaml
 traefik:
-  additional_ports: 
+  ports: 
     - 9000
     - 9001
   args:
@@ -455,7 +455,8 @@ labels:
   traefik.http.services.myservice.loadbalancer.server.port: 8080
 
 traefik:
-  additional_ports:
+  ports:
+    - 80
     - 9000
   args:
     'entrypoints.web.address=:80': true
diff --git a/lib/mrsk/commands/traefik.rb b/lib/mrsk/commands/traefik.rb
index acda3cf5..a6e2e229 100644
--- a/lib/mrsk/commands/traefik.rb
+++ b/lib/mrsk/commands/traefik.rb
@@ -8,8 +8,7 @@ class Mrsk::Commands::Traefik < Mrsk::Commands::Base
       "--detach",
       "--restart", "unless-stopped",
       "--log-opt", "max-size=#{MAX_LOG_SIZE}",
-      "--publish", port,
-      *additional_ports,
+      *published_ports,
       "--volume", "/var/run/docker.sock:/var/run/docker.sock",
       "traefik",
       "--providers.docker",
@@ -55,11 +54,11 @@ class Mrsk::Commands::Traefik < Mrsk::Commands::Base
   end
 
   private
-    def additional_ports
-      if args = config.raw_config.dig(:traefik, "additional_ports")
+    def published_ports
+      if args = config.raw_config.dig(:traefik, "ports")
         args.collect { |value| "--publish #{value}:#{value}" }.compact
       else
-        []
+        ["--publish #{port}"]
       end
     end
 
diff --git a/test/commands/traefik_test.rb b/test/commands/traefik_test.rb
index 58d0a557..300565c0 100644
--- a/test/commands/traefik_test.rb
+++ b/test/commands/traefik_test.rb
@@ -19,14 +19,14 @@ class CommandsTraefikTest < ActiveSupport::TestCase
       new_command.run.join(" ")
   end
 
-  test "run with additional entrypoints" do
+  test "run with ports configured" do
     assert_equal \
       "docker run --name traefik --detach --restart unless-stopped --log-opt max-size=10m --publish 80:80 --volume /var/run/docker.sock:/var/run/docker.sock traefik --providers.docker --log.level=DEBUG --accesslog.format \"json\" --metrics.prometheus.buckets \"0.1,0.3,1.2,5.0\"",
       new_command.run.join(" ")
 
-    @config[:traefik]["additional_ports"] = %w[9000 9001]
+    @config[:traefik]["ports"] = %w[9000 9001]
     assert_equal \
-      "docker run --name traefik --detach --restart unless-stopped --log-opt max-size=10m --publish 80:80 --publish 9000:9000 --publish 9001:9001 --volume /var/run/docker.sock:/var/run/docker.sock traefik --providers.docker --log.level=DEBUG --accesslog.format \"json\" --metrics.prometheus.buckets \"0.1,0.3,1.2,5.0\"",
+      "docker run --name traefik --detach --restart unless-stopped --log-opt max-size=10m --publish 9000:9000 --publish 9001:9001 --volume /var/run/docker.sock:/var/run/docker.sock traefik --providers.docker --log.level=DEBUG --accesslog.format \"json\" --metrics.prometheus.buckets \"0.1,0.3,1.2,5.0\"",
       new_command.run.join(" ")
   end
 

From 2db1bfde00ea2c6223eb204a49f67f249bcfc3fa Mon Sep 17 00:00:00 2001
From: Stephen van Beek <stephen@happyvalley.io>
Date: Tue, 14 Mar 2023 19:55:37 +0000
Subject: [PATCH 03/19] Added volume configuration in response to issue coments

---
 README.md                     | 35 ++++++++++++++++++++++++-----------
 lib/mrsk/commands/traefik.rb  | 14 +++++++++++---
 test/commands/traefik_test.rb | 24 +++++++++++++++++++++++-
 3 files changed, 58 insertions(+), 15 deletions(-)

diff --git a/README.md b/README.md
index 448d8aaf..c84d34ad 100644
--- a/README.md
+++ b/README.md
@@ -426,21 +426,31 @@ traefik:
   host_port: 8080
 ```
 
-### Configure entrypoints for traefik
+### Configure docker options for traefik
 
-You can override the ports and entrypoints for traefik list so:
+We allow users to override the published ports and bound volumes for the traefik container like so:
 
 ```yaml
 traefik:
-  ports: 
+  options:
+    publish:
     - 9000
-    - 9001
-  args:
-    entrypoints.myentrypoint.address: ":9000"
-    entrypoints.otherentrypoint.address: ":9001"
+    - 80    
 ```
 
-Be aware, a lot of the out-of-the-box magic of mrsk is provided by traefik defaults so going down this path requires more manual configuration, like so:
+Note, this fully overrides any defaults. If you choose to do this, then you'll like need to start out by copying the 
+default configuration:
+
+```yaml
+traefik:
+  options: 
+    publish:
+    - 80
+    volumes:
+      - /var/run/docker.sock:/var/run/docker.sock
+  args:
+    'entrypoints.web.address=:80': true
+```
 
 A more complete example including entrypoints would be:
 
@@ -455,9 +465,12 @@ labels:
   traefik.http.services.myservice.loadbalancer.server.port: 8080
 
 traefik:
-  ports:
-    - 80
-    - 9000
+  options:
+    publish:
+      - 80
+      - 9000
+    volumes:
+      - /var/run/docker.sock:/var/run/docker.sock
   args:
     'entrypoints.web.address=:80': true
     'entrypoints.otherentrypoint.address=:9000': true
diff --git a/lib/mrsk/commands/traefik.rb b/lib/mrsk/commands/traefik.rb
index a6e2e229..65654751 100644
--- a/lib/mrsk/commands/traefik.rb
+++ b/lib/mrsk/commands/traefik.rb
@@ -9,7 +9,7 @@ class Mrsk::Commands::Traefik < Mrsk::Commands::Base
       "--restart", "unless-stopped",
       "--log-opt", "max-size=#{MAX_LOG_SIZE}",
       *published_ports,
-      "--volume", "/var/run/docker.sock:/var/run/docker.sock",
+     *volumes,
       "traefik",
       "--providers.docker",
       "--log.level=DEBUG",
@@ -55,13 +55,21 @@ class Mrsk::Commands::Traefik < Mrsk::Commands::Base
 
   private
     def published_ports
-      if args = config.raw_config.dig(:traefik, "ports")
-        args.collect { |value| "--publish #{value}:#{value}" }.compact
+      if ports = config.raw_config.dig(:traefik, "options", "publish")
+        ports.collect { |value| "--publish #{value}:#{value}" }.compact
       else
         ["--publish #{port}"]
       end
     end
 
+    def volumes
+      if volumes = config.raw_config.dig(:traefik, "options", "volumes")
+        volumes.collect { |value| "--volume #{value}" }.compact
+      else
+        ["--volume /var/run/docker.sock:/var/run/docker.sock"]
+      end
+    end
+
     def cmd_option_args
       if args = config.traefik["args"]
         optionize args, with: "="
diff --git a/test/commands/traefik_test.rb b/test/commands/traefik_test.rb
index 300565c0..a3f1adb2 100644
--- a/test/commands/traefik_test.rb
+++ b/test/commands/traefik_test.rb
@@ -24,12 +24,34 @@ class CommandsTraefikTest < ActiveSupport::TestCase
       "docker run --name traefik --detach --restart unless-stopped --log-opt max-size=10m --publish 80:80 --volume /var/run/docker.sock:/var/run/docker.sock traefik --providers.docker --log.level=DEBUG --accesslog.format \"json\" --metrics.prometheus.buckets \"0.1,0.3,1.2,5.0\"",
       new_command.run.join(" ")
 
-    @config[:traefik]["ports"] = %w[9000 9001]
+    @config[:traefik]["options"] = {"publish" => [9000, 9001]}
     assert_equal \
       "docker run --name traefik --detach --restart unless-stopped --log-opt max-size=10m --publish 9000:9000 --publish 9001:9001 --volume /var/run/docker.sock:/var/run/docker.sock traefik --providers.docker --log.level=DEBUG --accesslog.format \"json\" --metrics.prometheus.buckets \"0.1,0.3,1.2,5.0\"",
       new_command.run.join(" ")
   end
 
+  test "run with volumes configured" do
+    assert_equal \
+      "docker run --name traefik --detach --restart unless-stopped --log-opt max-size=10m --publish 80:80 --volume /var/run/docker.sock:/var/run/docker.sock traefik --providers.docker --log.level=DEBUG --accesslog.format \"json\" --metrics.prometheus.buckets \"0.1,0.3,1.2,5.0\"",
+      new_command.run.join(" ")
+
+    @config[:traefik]["options"] = {"volumes" => %w[/var/run/docker.sock:/var/run/docker.sock ./letsencrypt/acme.json:/letsencrypt/acme.json] }
+    assert_equal \
+      "docker run --name traefik --detach --restart unless-stopped --log-opt max-size=10m --publish 80:80 --volume /var/run/docker.sock:/var/run/docker.sock --volume ./letsencrypt/acme.json:/letsencrypt/acme.json traefik --providers.docker --log.level=DEBUG --accesslog.format \"json\" --metrics.prometheus.buckets \"0.1,0.3,1.2,5.0\"",
+      new_command.run.join(" ")
+  end
+
+  test "run with ports and volumes configured" do
+    assert_equal \
+      "docker run --name traefik --detach --restart unless-stopped --log-opt max-size=10m --publish 80:80 --volume /var/run/docker.sock:/var/run/docker.sock traefik --providers.docker --log.level=DEBUG --accesslog.format \"json\" --metrics.prometheus.buckets \"0.1,0.3,1.2,5.0\"",
+      new_command.run.join(" ")
+
+    @config[:traefik]["options"] = {"volumes" => %w[/var/run/docker.sock:/var/run/docker.sock ./letsencrypt/acme.json:/letsencrypt/acme.json], "publish" => [80, 8080] }
+    assert_equal \
+      "docker run --name traefik --detach --restart unless-stopped --log-opt max-size=10m --publish 80:80 --publish 8080:8080 --volume /var/run/docker.sock:/var/run/docker.sock --volume ./letsencrypt/acme.json:/letsencrypt/acme.json traefik --providers.docker --log.level=DEBUG --accesslog.format \"json\" --metrics.prometheus.buckets \"0.1,0.3,1.2,5.0\"",
+      new_command.run.join(" ")
+  end
+
   test "run without configuration" do
     @config.delete(:traefik)
 

From 53046efad4f5624ee8a942242bfa53d82a604be6 Mon Sep 17 00:00:00 2001
From: Stephen van Beek <stephen@happyvalley.io>
Date: Tue, 14 Mar 2023 20:11:09 +0000
Subject: [PATCH 04/19] Rebased on main

---
 README.md                     |  6 +++---
 test/commands/traefik_test.rb | 12 ++++++------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/README.md b/README.md
index c84d34ad..b5b65359 100644
--- a/README.md
+++ b/README.md
@@ -449,7 +449,7 @@ traefik:
     volumes:
       - /var/run/docker.sock:/var/run/docker.sock
   args:
-    'entrypoints.web.address=:80': true
+    entrypoints.web.address: ':80'
 ```
 
 A more complete example including entrypoints would be:
@@ -472,8 +472,8 @@ traefik:
     volumes:
       - /var/run/docker.sock:/var/run/docker.sock
   args:
-    'entrypoints.web.address=:80': true
-    'entrypoints.otherentrypoint.address=:9000': true
+    entrypoints.web.address: ':80'
+    entrypoints.otherentrypoint.address: ':9000'
 ```
 
 ### Configuring build args for new images
diff --git a/test/commands/traefik_test.rb b/test/commands/traefik_test.rb
index a3f1adb2..1d0cf17a 100644
--- a/test/commands/traefik_test.rb
+++ b/test/commands/traefik_test.rb
@@ -21,34 +21,34 @@ class CommandsTraefikTest < ActiveSupport::TestCase
 
   test "run with ports configured" do
     assert_equal \
-      "docker run --name traefik --detach --restart unless-stopped --log-opt max-size=10m --publish 80:80 --volume /var/run/docker.sock:/var/run/docker.sock traefik --providers.docker --log.level=DEBUG --accesslog.format \"json\" --metrics.prometheus.buckets \"0.1,0.3,1.2,5.0\"",
+      "docker run --name traefik --detach --restart unless-stopped --log-opt max-size=10m --publish 80:80 --volume /var/run/docker.sock:/var/run/docker.sock traefik --providers.docker --log.level=DEBUG --accesslog.format=\"json\" --api.insecure --metrics.prometheus.buckets=\"0.1,0.3,1.2,5.0\"",
       new_command.run.join(" ")
 
     @config[:traefik]["options"] = {"publish" => [9000, 9001]}
     assert_equal \
-      "docker run --name traefik --detach --restart unless-stopped --log-opt max-size=10m --publish 9000:9000 --publish 9001:9001 --volume /var/run/docker.sock:/var/run/docker.sock traefik --providers.docker --log.level=DEBUG --accesslog.format \"json\" --metrics.prometheus.buckets \"0.1,0.3,1.2,5.0\"",
+      "docker run --name traefik --detach --restart unless-stopped --log-opt max-size=10m --publish 9000:9000 --publish 9001:9001 --volume /var/run/docker.sock:/var/run/docker.sock traefik --providers.docker --log.level=DEBUG --accesslog.format=\"json\" --api.insecure --metrics.prometheus.buckets=\"0.1,0.3,1.2,5.0\"",
       new_command.run.join(" ")
   end
 
   test "run with volumes configured" do
     assert_equal \
-      "docker run --name traefik --detach --restart unless-stopped --log-opt max-size=10m --publish 80:80 --volume /var/run/docker.sock:/var/run/docker.sock traefik --providers.docker --log.level=DEBUG --accesslog.format \"json\" --metrics.prometheus.buckets \"0.1,0.3,1.2,5.0\"",
+      "docker run --name traefik --detach --restart unless-stopped --log-opt max-size=10m --publish 80:80 --volume /var/run/docker.sock:/var/run/docker.sock traefik --providers.docker --log.level=DEBUG --accesslog.format=\"json\" --api.insecure --metrics.prometheus.buckets=\"0.1,0.3,1.2,5.0\"",
       new_command.run.join(" ")
 
     @config[:traefik]["options"] = {"volumes" => %w[/var/run/docker.sock:/var/run/docker.sock ./letsencrypt/acme.json:/letsencrypt/acme.json] }
     assert_equal \
-      "docker run --name traefik --detach --restart unless-stopped --log-opt max-size=10m --publish 80:80 --volume /var/run/docker.sock:/var/run/docker.sock --volume ./letsencrypt/acme.json:/letsencrypt/acme.json traefik --providers.docker --log.level=DEBUG --accesslog.format \"json\" --metrics.prometheus.buckets \"0.1,0.3,1.2,5.0\"",
+      "docker run --name traefik --detach --restart unless-stopped --log-opt max-size=10m --publish 80:80 --volume /var/run/docker.sock:/var/run/docker.sock --volume ./letsencrypt/acme.json:/letsencrypt/acme.json traefik --providers.docker --log.level=DEBUG --accesslog.format=\"json\" --api.insecure --metrics.prometheus.buckets=\"0.1,0.3,1.2,5.0\"",
       new_command.run.join(" ")
   end
 
   test "run with ports and volumes configured" do
     assert_equal \
-      "docker run --name traefik --detach --restart unless-stopped --log-opt max-size=10m --publish 80:80 --volume /var/run/docker.sock:/var/run/docker.sock traefik --providers.docker --log.level=DEBUG --accesslog.format \"json\" --metrics.prometheus.buckets \"0.1,0.3,1.2,5.0\"",
+      "docker run --name traefik --detach --restart unless-stopped --log-opt max-size=10m --publish 80:80 --volume /var/run/docker.sock:/var/run/docker.sock traefik --providers.docker --log.level=DEBUG --accesslog.format=\"json\" --api.insecure --metrics.prometheus.buckets=\"0.1,0.3,1.2,5.0\"",
       new_command.run.join(" ")
 
     @config[:traefik]["options"] = {"volumes" => %w[/var/run/docker.sock:/var/run/docker.sock ./letsencrypt/acme.json:/letsencrypt/acme.json], "publish" => [80, 8080] }
     assert_equal \
-      "docker run --name traefik --detach --restart unless-stopped --log-opt max-size=10m --publish 80:80 --publish 8080:8080 --volume /var/run/docker.sock:/var/run/docker.sock --volume ./letsencrypt/acme.json:/letsencrypt/acme.json traefik --providers.docker --log.level=DEBUG --accesslog.format \"json\" --metrics.prometheus.buckets \"0.1,0.3,1.2,5.0\"",
+      "docker run --name traefik --detach --restart unless-stopped --log-opt max-size=10m --publish 80:80 --publish 8080:8080 --volume /var/run/docker.sock:/var/run/docker.sock --volume ./letsencrypt/acme.json:/letsencrypt/acme.json traefik --providers.docker --log.level=DEBUG --accesslog.format=\"json\" --api.insecure --metrics.prometheus.buckets=\"0.1,0.3,1.2,5.0\"",
       new_command.run.join(" ")
   end
 

From c2ca269eb6943194dd9a8917106d54d0a8bda4b2 Mon Sep 17 00:00:00 2001
From: Stephen van Beek <stephen@happyvalley.io>
Date: Tue, 14 Mar 2023 20:12:11 +0000
Subject: [PATCH 05/19] Fixed readme

---
 README.md | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/README.md b/README.md
index b5b65359..fcfeb439 100644
--- a/README.md
+++ b/README.md
@@ -436,6 +436,8 @@ traefik:
     publish:
     - 9000
     - 80    
+    volumes:
+      - /tmp/example:/tmp/example
 ```
 
 Note, this fully overrides any defaults. If you choose to do this, then you'll like need to start out by copying the 

From 9843c5e1ce8f7ac895340e6f6a3a6277776826ba Mon Sep 17 00:00:00 2001
From: Stephen van Beek <stephen@happyvalley.io>
Date: Tue, 14 Mar 2023 20:13:13 +0000
Subject: [PATCH 06/19] Fixed typos

---
 README.md                    | 2 +-
 lib/mrsk/commands/traefik.rb | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/README.md b/README.md
index fcfeb439..da56ed0a 100644
--- a/README.md
+++ b/README.md
@@ -449,7 +449,7 @@ traefik:
     publish:
     - 80
     volumes:
-      - /var/run/docker.sock:/var/run/docker.sock
+    - /var/run/docker.sock:/var/run/docker.sock
   args:
     entrypoints.web.address: ':80'
 ```
diff --git a/lib/mrsk/commands/traefik.rb b/lib/mrsk/commands/traefik.rb
index 65654751..f6017004 100644
--- a/lib/mrsk/commands/traefik.rb
+++ b/lib/mrsk/commands/traefik.rb
@@ -9,7 +9,7 @@ class Mrsk::Commands::Traefik < Mrsk::Commands::Base
       "--restart", "unless-stopped",
       "--log-opt", "max-size=#{MAX_LOG_SIZE}",
       *published_ports,
-     *volumes,
+      *volumes,
       "traefik",
       "--providers.docker",
       "--log.level=DEBUG",

From 4c542930c56ff1ab92fb15c97c7b926c6b9343ce Mon Sep 17 00:00:00 2001
From: Stephen van Beek <stephen@happyvalley.io>
Date: Wed, 15 Mar 2023 15:37:10 +0000
Subject: [PATCH 07/19] Allow arbitrary docker options for traefik

---
 README.md                     | 34 +++++++++++-----------------------
 lib/mrsk/commands/traefik.rb  | 21 +++++++--------------
 lib/mrsk/utils.rb             | 10 ++++++++--
 test/commands/traefik_test.rb | 14 +++++++-------
 4 files changed, 33 insertions(+), 46 deletions(-)

diff --git a/README.md b/README.md
index da56ed0a..1293e9da 100644
--- a/README.md
+++ b/README.md
@@ -428,33 +428,24 @@ traefik:
 
 ### Configure docker options for traefik
 
-We allow users to override the published ports and bound volumes for the traefik container like so:
-
-```yaml
-traefik:
-  options:
-    publish:
-    - 9000
-    - 80    
-    volumes:
-      - /tmp/example:/tmp/example
-```
-
-Note, this fully overrides any defaults. If you choose to do this, then you'll like need to start out by copying the 
-default configuration:
+We allow users to pass additional docker options to the trafik container like 
 
 ```yaml
 traefik:
   options: 
     publish:
-    - 80
+    - 8080:8080
     volumes:
-    - /var/run/docker.sock:/var/run/docker.sock
-  args:
-    entrypoints.web.address: ':80'
+    - /tmp/example.json:/tmp/example.json
+    memory: 512m
 ```
 
-A more complete example including entrypoints would be:
+This will start the traefik container with a command like: `docker run ... --volume /tmp/example.json:/tmp/example.json --publish 8080:8080 `
+
+
+### Configure alternate entrypoints for traefik
+
+You can configure multiple entrypoints for traefik like so:
 
 ```yaml
 service: myservice
@@ -469,10 +460,7 @@ labels:
 traefik:
   options:
     publish:
-      - 80
-      - 9000
-    volumes:
-      - /var/run/docker.sock:/var/run/docker.sock
+      - 9000:9000
   args:
     entrypoints.web.address: ':80'
     entrypoints.otherentrypoint.address: ':9000'
diff --git a/lib/mrsk/commands/traefik.rb b/lib/mrsk/commands/traefik.rb
index f6017004..e09c527b 100644
--- a/lib/mrsk/commands/traefik.rb
+++ b/lib/mrsk/commands/traefik.rb
@@ -8,8 +8,9 @@ class Mrsk::Commands::Traefik < Mrsk::Commands::Base
       "--detach",
       "--restart", "unless-stopped",
       "--log-opt", "max-size=#{MAX_LOG_SIZE}",
-      *published_ports,
-      *volumes,
+      "--publish", port,
+      "--volume", "/var/run/docker.sock:/var/run/docker.sock",
+      *docker_option_args,
       "traefik",
       "--providers.docker",
       "--log.level=DEBUG",
@@ -54,19 +55,11 @@ class Mrsk::Commands::Traefik < Mrsk::Commands::Base
   end
 
   private
-    def published_ports
-      if ports = config.raw_config.dig(:traefik, "options", "publish")
-        ports.collect { |value| "--publish #{value}:#{value}" }.compact
+    def docker_option_args
+      if args = config.raw_config.dig(:traefik, "options")
+        optionize args
       else
-        ["--publish #{port}"]
-      end
-    end
-
-    def volumes
-      if volumes = config.raw_config.dig(:traefik, "options", "volumes")
-        volumes.collect { |value| "--volume #{value}" }.compact
-      else
-        ["--volume /var/run/docker.sock:/var/run/docker.sock"]
+        []
       end
     end
 
diff --git a/lib/mrsk/utils.rb b/lib/mrsk/utils.rb
index 1e763250..249e2ded 100644
--- a/lib/mrsk/utils.rb
+++ b/lib/mrsk/utils.rb
@@ -25,15 +25,21 @@ module Mrsk::Utils
 
   # Returns a list of shell-dashed option arguments. If the value is true, it's treated like a value-less option.
   def optionize(args, with: nil)
+    flattened_args = flatten_args args
     options = if with
-      args.collect { |(key, value)| value == true ? "--#{key}" : "--#{key}#{with}#{escape_shell_value(value)}" }
+      flattened_args.collect { |(key, value)| value == true ? "--#{key}" : "--#{key}#{with}#{escape_shell_value(value)}" }
     else
-      args.collect { |(key, value)| [ "--#{key}", value == true ? nil : escape_shell_value(value) ] }
+      flattened_args.collect { |(key, value)| [ "--#{key}", value == true ? nil : escape_shell_value(value) ] }
     end
 
     options.flatten.compact
   end
 
+  # Flattens a one-to-many structure into an array of two-element arrays each containing a key-value pair
+  def flatten_args(args)
+    args.flat_map { |key, value| value.respond_to?('map') ? value.map { |entry| [key, entry] }: [[key, value]] }
+  end
+
   # Copied from SSHKit::Backend::Abstract#redact to be available inside Commands classes
   def redact(arg) # Used in execute_command to hide redact() args a user passes in
     arg.to_s.extend(SSHKit::Redaction) # to_s due to our inability to extend Integer, etc
diff --git a/test/commands/traefik_test.rb b/test/commands/traefik_test.rb
index 1d0cf17a..f2b67f91 100644
--- a/test/commands/traefik_test.rb
+++ b/test/commands/traefik_test.rb
@@ -24,9 +24,9 @@ class CommandsTraefikTest < ActiveSupport::TestCase
       "docker run --name traefik --detach --restart unless-stopped --log-opt max-size=10m --publish 80:80 --volume /var/run/docker.sock:/var/run/docker.sock traefik --providers.docker --log.level=DEBUG --accesslog.format=\"json\" --api.insecure --metrics.prometheus.buckets=\"0.1,0.3,1.2,5.0\"",
       new_command.run.join(" ")
 
-    @config[:traefik]["options"] = {"publish" => [9000, 9001]}
+    @config[:traefik]["options"] = {"publish" => %w[9000:9000 9001:9001]}
     assert_equal \
-      "docker run --name traefik --detach --restart unless-stopped --log-opt max-size=10m --publish 9000:9000 --publish 9001:9001 --volume /var/run/docker.sock:/var/run/docker.sock traefik --providers.docker --log.level=DEBUG --accesslog.format=\"json\" --api.insecure --metrics.prometheus.buckets=\"0.1,0.3,1.2,5.0\"",
+      "docker run --name traefik --detach --restart unless-stopped --log-opt max-size=10m --publish 80:80 --volume /var/run/docker.sock:/var/run/docker.sock --publish \"9000:9000\" --publish \"9001:9001\" traefik --providers.docker --log.level=DEBUG --accesslog.format=\"json\" --api.insecure --metrics.prometheus.buckets=\"0.1,0.3,1.2,5.0\"",
       new_command.run.join(" ")
   end
 
@@ -35,20 +35,20 @@ class CommandsTraefikTest < ActiveSupport::TestCase
       "docker run --name traefik --detach --restart unless-stopped --log-opt max-size=10m --publish 80:80 --volume /var/run/docker.sock:/var/run/docker.sock traefik --providers.docker --log.level=DEBUG --accesslog.format=\"json\" --api.insecure --metrics.prometheus.buckets=\"0.1,0.3,1.2,5.0\"",
       new_command.run.join(" ")
 
-    @config[:traefik]["options"] = {"volumes" => %w[/var/run/docker.sock:/var/run/docker.sock ./letsencrypt/acme.json:/letsencrypt/acme.json] }
+    @config[:traefik]["options"] = {"volume" => %w[./letsencrypt/acme.json:/letsencrypt/acme.json] }
     assert_equal \
-      "docker run --name traefik --detach --restart unless-stopped --log-opt max-size=10m --publish 80:80 --volume /var/run/docker.sock:/var/run/docker.sock --volume ./letsencrypt/acme.json:/letsencrypt/acme.json traefik --providers.docker --log.level=DEBUG --accesslog.format=\"json\" --api.insecure --metrics.prometheus.buckets=\"0.1,0.3,1.2,5.0\"",
+      "docker run --name traefik --detach --restart unless-stopped --log-opt max-size=10m --publish 80:80 --volume /var/run/docker.sock:/var/run/docker.sock --volume \"./letsencrypt/acme.json:/letsencrypt/acme.json\" traefik --providers.docker --log.level=DEBUG --accesslog.format=\"json\" --api.insecure --metrics.prometheus.buckets=\"0.1,0.3,1.2,5.0\"",
       new_command.run.join(" ")
   end
 
-  test "run with ports and volumes configured" do
+  test "run with several options configured" do
     assert_equal \
       "docker run --name traefik --detach --restart unless-stopped --log-opt max-size=10m --publish 80:80 --volume /var/run/docker.sock:/var/run/docker.sock traefik --providers.docker --log.level=DEBUG --accesslog.format=\"json\" --api.insecure --metrics.prometheus.buckets=\"0.1,0.3,1.2,5.0\"",
       new_command.run.join(" ")
 
-    @config[:traefik]["options"] = {"volumes" => %w[/var/run/docker.sock:/var/run/docker.sock ./letsencrypt/acme.json:/letsencrypt/acme.json], "publish" => [80, 8080] }
+    @config[:traefik]["options"] = {"volume" => %w[./letsencrypt/acme.json:/letsencrypt/acme.json], "publish" => %w[8080:8080], "memory" => "512m"}
     assert_equal \
-      "docker run --name traefik --detach --restart unless-stopped --log-opt max-size=10m --publish 80:80 --publish 8080:8080 --volume /var/run/docker.sock:/var/run/docker.sock --volume ./letsencrypt/acme.json:/letsencrypt/acme.json traefik --providers.docker --log.level=DEBUG --accesslog.format=\"json\" --api.insecure --metrics.prometheus.buckets=\"0.1,0.3,1.2,5.0\"",
+      "docker run --name traefik --detach --restart unless-stopped --log-opt max-size=10m --publish 80:80 --volume /var/run/docker.sock:/var/run/docker.sock --volume \"./letsencrypt/acme.json:/letsencrypt/acme.json\" --publish \"8080:8080\" --memory \"512m\" traefik --providers.docker --log.level=DEBUG --accesslog.format=\"json\" --api.insecure --metrics.prometheus.buckets=\"0.1,0.3,1.2,5.0\"",
       new_command.run.join(" ")
   end
 

From c0d5b48f22500d06e5b5977f0bbcf50a59d6e868 Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremydaer@gmail.com>
Date: Wed, 22 Mar 2023 10:38:36 -0700
Subject: [PATCH 08/19] Polish destination config loading

* `Pathname#sub_ext` to munge .yml ext to .destination.yml
* Extract multi-file config merge
---
 lib/mrsk/configuration.rb | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/lib/mrsk/configuration.rb b/lib/mrsk/configuration.rb
index 03413116..163ded25 100644
--- a/lib/mrsk/configuration.rb
+++ b/lib/mrsk/configuration.rb
@@ -15,15 +15,16 @@ class Mrsk::Configuration
 
   class << self
     def create_from(base_config_file, destination: nil, version: "missing")
-      new(load_config_file(base_config_file).tap do |config|
-        if destination
-          config.deep_merge! \
-            load_config_file destination_config_file(base_config_file, destination)
-        end
-      end, destination: destination, version: version)
+      raw_config = load_config_files(base_config_file, *destination_config_file(base_config_file, destination))
+
+      new raw_config, destination: destination, version: version
     end
 
     private
+      def load_config_files(*files)
+        files.inject({}) { |config, file| config.deep_merge! load_config_file(file) }
+      end
+
       def load_config_file(file)
         if file.exist?
           YAML.load(ERB.new(IO.read(file)).result).symbolize_keys
@@ -33,8 +34,7 @@ class Mrsk::Configuration
       end
 
       def destination_config_file(base_config_file, destination)
-        dir, basename = base_config_file.split
-        dir.join basename.to_s.remove(".yml") + ".#{destination}.yml"
+        base_config_file.sub_ext(".#{destination}.yml") if destination
       end
   end
 

From 04b1d5e49ea9bc32cbf0971ceb73eefd1c9b05d7 Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@hey.com>
Date: Wed, 22 Mar 2023 21:07:04 -0700
Subject: [PATCH 09/19] Accessories may be pulled from authenticated registries

---
 README.md                       |  8 +++++++-
 lib/mrsk/cli/accessory.rb       |  1 +
 test/cli/accessory_test.rb      |  3 +++
 test/commands/accessory_test.rb | 10 +++++-----
 4 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/README.md b/README.md
index 56d26116..eaa17aea 100644
--- a/README.md
+++ b/README.md
@@ -445,7 +445,7 @@ FROM ruby:$RUBY_VERSION-slim as base
 
 ### Using accessories for database, cache, search services
 
-You can manage your accessory services via MRSK as well. The services will build off public images, and will not be automatically updated when you deploy:
+You can manage your accessory services via MRSK as well. Accessories are long-lived services that your app depends on. They are not updated when you deploy.
 
 ```yaml
 accessories:
@@ -466,10 +466,16 @@ accessories:
     port: "36379:6379"
     volumes:
       - /var/lib/redis:/data
+  internal-example:
+    image: registry.digitalocean.com/user/otherservice:latest
+    host: 1.1.1.5
+    port: 44444
 ```
 
 Now run `mrsk accessory start mysql` to start the MySQL server on 1.1.1.3. See `mrsk accessory` for all the commands possible.
 
+Accessory images must be public or tagged in your private registry.
+
 ### Using Cron
 
 You can use a specific container to run your Cron jobs:
diff --git a/lib/mrsk/cli/accessory.rb b/lib/mrsk/cli/accessory.rb
index 834c7dec..36f5cd3c 100644
--- a/lib/mrsk/cli/accessory.rb
+++ b/lib/mrsk/cli/accessory.rb
@@ -9,6 +9,7 @@ class Mrsk::Cli::Accessory < Mrsk::Cli::Base
         upload(name)
 
         on(accessory.host) do
+          execute *MRSK.registry.login
           execute *MRSK.auditor.record("Booted #{name} accessory"), verbosity: :debug
           execute *accessory.run
         end
diff --git a/test/cli/accessory_test.rb b/test/cli/accessory_test.rb
index fc6c25fb..8d2fe435 100644
--- a/test/cli/accessory_test.rb
+++ b/test/cli/accessory_test.rb
@@ -6,6 +6,7 @@ class CliAccessoryTest < CliTestCase
     Mrsk::Cli::Accessory.any_instance.expects(:upload).with("mysql")
 
     run_command("boot", "mysql").tap do |output|
+      assert_match /docker login.*on 1.1.1.3/, output
       assert_match "docker run --name app-mysql --detach --restart unless-stopped --log-opt max-size=10m --publish 3306:3306 -e [REDACTED] -e MYSQL_ROOT_HOST=\"%\" --volume $PWD/app-mysql/etc/mysql/my.cnf:/etc/mysql/my.cnf --volume $PWD/app-mysql/data:/var/lib/mysql --label service=\"app-mysql\" mysql:5.7 on 1.1.1.3", output
     end
   end
@@ -17,6 +18,8 @@ class CliAccessoryTest < CliTestCase
     Mrsk::Cli::Accessory.any_instance.expects(:upload).with("redis")
 
     run_command("boot", "all").tap do |output|
+      assert_match /docker login.*on 1.1.1.3/, output
+      assert_match /docker login.*on 1.1.1.4/, output
       assert_match "docker run --name app-mysql --detach --restart unless-stopped --log-opt max-size=10m --publish 3306:3306 -e [REDACTED] -e MYSQL_ROOT_HOST=\"%\" --volume $PWD/app-mysql/etc/mysql/my.cnf:/etc/mysql/my.cnf --volume $PWD/app-mysql/data:/var/lib/mysql --label service=\"app-mysql\" mysql:5.7 on 1.1.1.3", output
       assert_match "docker run --name app-redis --detach --restart unless-stopped --log-opt max-size=10m --publish 6379:6379 --volume $PWD/app-redis/data:/data --label service=\"app-redis\" redis:latest on 1.1.1.4", output
     end
diff --git a/test/commands/accessory_test.rb b/test/commands/accessory_test.rb
index c98e1590..156d5794 100644
--- a/test/commands/accessory_test.rb
+++ b/test/commands/accessory_test.rb
@@ -3,11 +3,11 @@ require "test_helper"
 class CommandsAccessoryTest < ActiveSupport::TestCase
   setup do
     @config = {
-      service: "app", image: "dhh/app", registry: { "username" => "dhh", "password" => "secret" },
+      service: "app", image: "dhh/app", registry: { "server" => "private.registry", "username" => "dhh", "password" => "secret" },
       servers: [ "1.1.1.1" ],
       accessories: {
         "mysql" => {
-          "image" => "mysql:8.0",
+          "image" => "private.registry/mysql:8.0",
           "host" => "1.1.1.5",
           "port" => "3306",
           "env" => {
@@ -49,7 +49,7 @@ class CommandsAccessoryTest < ActiveSupport::TestCase
 
   test "run" do
     assert_equal \
-      "docker run --name app-mysql --detach --restart unless-stopped --log-opt max-size=10m --publish 3306:3306 -e MYSQL_ROOT_PASSWORD=\"secret123\" -e MYSQL_ROOT_HOST=\"%\" --label service=\"app-mysql\" mysql:8.0",
+      "docker run --name app-mysql --detach --restart unless-stopped --log-opt max-size=10m --publish 3306:3306 -e MYSQL_ROOT_PASSWORD=\"secret123\" -e MYSQL_ROOT_HOST=\"%\" --label service=\"app-mysql\" private.registry/mysql:8.0",
       @mysql.run.join(" ")
 
     assert_equal \
@@ -78,7 +78,7 @@ class CommandsAccessoryTest < ActiveSupport::TestCase
 
   test "execute in new container" do
     assert_equal \
-      "docker run --rm -e MYSQL_ROOT_PASSWORD=\"secret123\" -e MYSQL_ROOT_HOST=\"%\" mysql:8.0 mysql -u root",
+      "docker run --rm -e MYSQL_ROOT_PASSWORD=\"secret123\" -e MYSQL_ROOT_HOST=\"%\" private.registry/mysql:8.0 mysql -u root",
       @mysql.execute_in_new_container("mysql", "-u", "root").join(" ")
   end
 
@@ -90,7 +90,7 @@ class CommandsAccessoryTest < ActiveSupport::TestCase
 
   test "execute in new container over ssh" do
     @mysql.stub(:run_over_ssh, ->(cmd) { cmd.join(" ") }) do
-      assert_match %r|docker run -it --rm -e MYSQL_ROOT_PASSWORD=\"secret123\" -e MYSQL_ROOT_HOST=\"%\" mysql:8.0 mysql -u root|,
+      assert_match %r|docker run -it --rm -e MYSQL_ROOT_PASSWORD=\"secret123\" -e MYSQL_ROOT_HOST=\"%\" private.registry/mysql:8.0 mysql -u root|,
         @mysql.execute_in_new_container_over_ssh("mysql", "-u", "root")
     end
   end

From c870e560c11757dfa50d3e7e1ce74a20fa6d9bb4 Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@hey.com>
Date: Thu, 23 Mar 2023 00:10:18 -0700
Subject: [PATCH 10/19] Accessories aren't required to publish a port

Allows for background accessories like schedulers that don't act
as typical network service dependencies and have no port to expose.
---
 lib/mrsk/commands/accessory.rb      |  4 ++--
 lib/mrsk/configuration/accessory.rb | 10 ++++++----
 test/commands/accessory_test.rb     | 15 ++++++++++++---
 3 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/lib/mrsk/commands/accessory.rb b/lib/mrsk/commands/accessory.rb
index dbffc71b..76d4fa96 100644
--- a/lib/mrsk/commands/accessory.rb
+++ b/lib/mrsk/commands/accessory.rb
@@ -1,6 +1,6 @@
 class Mrsk::Commands::Accessory < Mrsk::Commands::Base
   attr_reader :accessory_config
-  delegate :service_name, :image, :host, :port, :files, :directories, :env_args, :volume_args, :label_args, to: :accessory_config
+  delegate :service_name, :image, :host, :port, :files, :directories, :publish_args, :env_args, :volume_args, :label_args, to: :accessory_config
 
   def initialize(config, name:)
     super(config)
@@ -13,7 +13,7 @@ class Mrsk::Commands::Accessory < Mrsk::Commands::Base
       "--detach",
       "--restart", "unless-stopped",
       "--log-opt", "max-size=#{MAX_LOG_SIZE}",
-      "--publish", port,
+      *publish_args,
       *env_args,
       *volume_args,
       *label_args,
diff --git a/lib/mrsk/configuration/accessory.rb b/lib/mrsk/configuration/accessory.rb
index 58c4b125..f1051423 100644
--- a/lib/mrsk/configuration/accessory.rb
+++ b/lib/mrsk/configuration/accessory.rb
@@ -20,13 +20,15 @@ class Mrsk::Configuration::Accessory
   end
 
   def port
-    if specifics["port"].to_s.include?(":")
-      specifics["port"]
-    else
-      "#{specifics["port"]}:#{specifics["port"]}"
+    if port = specifics["port"]&.to_s
+      port.include?(":") ? port : "#{port}:#{port}"
     end
   end
 
+  def publish_args
+    argumentize "--publish", port if port
+  end
+
   def labels
     default_labels.merge(specifics["labels"] || {})
   end
diff --git a/test/commands/accessory_test.rb b/test/commands/accessory_test.rb
index c98e1590..7a3d114d 100644
--- a/test/commands/accessory_test.rb
+++ b/test/commands/accessory_test.rb
@@ -32,13 +32,18 @@ class CommandsAccessoryTest < ActiveSupport::TestCase
           "volumes" => [
             "/var/lib/redis:/data"
           ]
+        },
+        "busybox" => {
+          "image" => "busybox:latest",
+          "host" => "1.1.1.7"
         }
       }
     }
 
-    @config = Mrsk::Configuration.new(@config)
-    @mysql  = Mrsk::Commands::Accessory.new(@config, name: :mysql)
-    @redis  = Mrsk::Commands::Accessory.new(@config, name: :redis)
+    @config  = Mrsk::Configuration.new(@config)
+    @mysql   = Mrsk::Commands::Accessory.new(@config, name: :mysql)
+    @redis   = Mrsk::Commands::Accessory.new(@config, name: :redis)
+    @busybox = Mrsk::Commands::Accessory.new(@config, name: :busybox)
 
     ENV["MYSQL_ROOT_PASSWORD"] = "secret123"
   end
@@ -55,6 +60,10 @@ class CommandsAccessoryTest < ActiveSupport::TestCase
     assert_equal \
       "docker run --name app-redis --detach --restart unless-stopped --log-opt max-size=10m --publish 6379:6379 -e SOMETHING=\"else\" --volume /var/lib/redis:/data --label service=\"app-redis\" --label cache=\"true\" redis:latest",
       @redis.run.join(" ")
+
+    assert_equal \
+      "docker run --name app-busybox --detach --restart unless-stopped --log-opt max-size=10m --label service=\"app-busybox\" busybox:latest",
+      @busybox.run.join(" ")
   end
 
   test "start" do

From 53d7f9d528615cb07707527f4ac54c59c5162198 Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremydaer@gmail.com>
Date: Wed, 22 Mar 2023 10:15:12 -0700
Subject: [PATCH 11/19] Deploys mention the released service@version

Less work for broadcast commands to take on.

Also fixes a bug where rollback on hosts without a running container
would stop the container they had just started.
---
 lib/mrsk/cli/main.rb      | 17 ++++++++++++-----
 lib/mrsk/configuration.rb |  5 +++++
 lib/mrsk/utils.rb         |  5 +++++
 test/cli/main_test.rb     | 18 +++++++++++++++---
 4 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/lib/mrsk/cli/main.rb b/lib/mrsk/cli/main.rb
index 328af480..1524b49e 100644
--- a/lib/mrsk/cli/main.rb
+++ b/lib/mrsk/cli/main.rb
@@ -40,7 +40,7 @@ class Mrsk::Cli::Main < Mrsk::Cli::Base
       invoke "mrsk:cli:prune:all", [], invoke_options
     end
 
-    audit_broadcast "Deployed app in #{runtime.to_i} seconds" unless options[:skip_broadcast]
+    audit_broadcast "Deployed #{service_version} in #{runtime.round} seconds" unless options[:skip_broadcast]
   end
 
   desc "redeploy", "Deploy app to servers without bootstrapping servers, starting Traefik, pruning, and registry login"
@@ -63,7 +63,7 @@ class Mrsk::Cli::Main < Mrsk::Cli::Base
       invoke "mrsk:cli:app:boot", [], invoke_options
     end
 
-    audit_broadcast "Redeployed app in #{runtime.to_i} seconds" unless options[:skip_broadcast]
+    audit_broadcast "Redeployed #{service_version} in #{runtime.round} seconds" unless options[:skip_broadcast]
   end
 
   desc "rollback [VERSION]", "Rollback app to VERSION"
@@ -74,18 +74,21 @@ class Mrsk::Cli::Main < Mrsk::Cli::Base
       say "Start version #{version}, then wait #{MRSK.config.readiness_delay}s for app to boot before stopping the old version...", :magenta
 
       cli = self
+      old_version = nil
 
       on(MRSK.hosts) do |host|
         old_version = capture_with_info(*MRSK.app.current_running_version).strip.presence
 
         execute *MRSK.app.start
 
-        sleep MRSK.config.readiness_delay
+        if old_version
+          sleep MRSK.config.readiness_delay
 
-        execute *MRSK.app.stop(version: old_version), raise_on_non_zero_exit: false
+          execute *MRSK.app.stop(version: old_version), raise_on_non_zero_exit: false
+        end
       end
 
-      audit_broadcast "Rolled back app to version #{version}" unless options[:skip_broadcast]
+      audit_broadcast "Rolled back #{service_version(Mrsk::Utils.abbreviate_version(old_version))} to #{service_version}" unless options[:skip_broadcast]
     else
       say "The app version '#{version}' is not available as a container (use 'mrsk app containers' for available versions)", :red
     end
@@ -203,4 +206,8 @@ class Mrsk::Cli::Main < Mrsk::Cli::Base
       on(host) { container_names = capture_with_info(*MRSK.app.list_container_names).split("\n") }
       Array(container_names).include?(container_name)
     end
+
+    def service_version(version = MRSK.config.abbreviated_version)
+      [ MRSK.config.service, version ].compact.join("@")
+    end
 end
diff --git a/lib/mrsk/configuration.rb b/lib/mrsk/configuration.rb
index 03413116..37ef8d4c 100644
--- a/lib/mrsk/configuration.rb
+++ b/lib/mrsk/configuration.rb
@@ -46,6 +46,11 @@ class Mrsk::Configuration
   end
 
 
+  def abbreviated_version
+    Mrsk::Utils.abbreviate_version(version)
+  end
+
+
   def roles
     @roles ||= role_names.collect { |role_name| Role.new(role_name, config: self) }
   end
diff --git a/lib/mrsk/utils.rb b/lib/mrsk/utils.rb
index 1e763250..4a726810 100644
--- a/lib/mrsk/utils.rb
+++ b/lib/mrsk/utils.rb
@@ -43,4 +43,9 @@ module Mrsk::Utils
   def escape_shell_value(value)
     value.to_s.dump.gsub(/`/, '\\\\`')
   end
+
+  # Abbreviate a git revhash for concise display
+  def abbreviate_version(version)
+    version[0...7] if version
+  end
 end
diff --git a/test/cli/main_test.rb b/test/cli/main_test.rb
index f72741dd..086067ff 100644
--- a/test/cli/main_test.rb
+++ b/test/cli/main_test.rb
@@ -88,11 +88,23 @@ class CliMainTest < CliTestCase
 
   test "rollback good version" do
     Mrsk::Cli::Main.any_instance.stubs(:container_name_available?).returns(true)
+    SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info).with(:docker, :ps, "--filter", "label=service=app", "--format", "\"{{.Names}}\"", "|", "sed 's/-/\\n/g'", "|", "tail -n 1").returns("version-to-rollback\n").times(2)
 
     run_command("rollback", "123").tap do |output|
-      assert_match /Start version 123/, output
-      assert_match /docker ps -q --filter label=service=app | xargs docker stop/, output
-      assert_match /docker start app-123/, output
+      assert_match "Start version 123", output
+      assert_match "docker start app-123", output
+      assert_match "docker container ls --all --filter name=app-version-to-rollback --quiet | xargs docker stop", output, "Should stop the container that was previously running"
+    end
+  end
+
+  test "rollback without old version" do
+    Mrsk::Cli::Main.any_instance.stubs(:container_name_available?).returns(true)
+    SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info).with(:docker, :ps, "--filter", "label=service=app", "--format", "\"{{.Names}}\"", "|", "sed 's/-/\\n/g'", "|", "tail -n 1").returns("").times(2)
+
+    run_command("rollback", "123").tap do |output|
+      assert_match "Start version 123", output
+      assert_match "docker start app-123", output
+      assert_no_match "docker stop", output
     end
   end
 

From f3d93d3899c27c8d8f1bcd33153407c77e44481f Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@hey.com>
Date: Wed, 22 Mar 2023 22:16:30 -0700
Subject: [PATCH 12/19] Bump dev deps and consolidate platform matches

---
 Gemfile.lock | 57 +++++++++++++++++++++-------------------------------
 1 file changed, 23 insertions(+), 34 deletions(-)

diff --git a/Gemfile.lock b/Gemfile.lock
index d79bedff..815761d0 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -13,90 +13,79 @@ PATH
 GEM
   remote: https://rubygems.org/
   specs:
-    actionpack (7.0.4)
-      actionview (= 7.0.4)
-      activesupport (= 7.0.4)
+    actionpack (7.0.4.3)
+      actionview (= 7.0.4.3)
+      activesupport (= 7.0.4.3)
       rack (~> 2.0, >= 2.2.0)
       rack-test (>= 0.6.3)
       rails-dom-testing (~> 2.0)
       rails-html-sanitizer (~> 1.0, >= 1.2.0)
-    actionview (7.0.4)
-      activesupport (= 7.0.4)
+    actionview (7.0.4.3)
+      activesupport (= 7.0.4.3)
       builder (~> 3.1)
       erubi (~> 1.4)
       rails-dom-testing (~> 2.0)
       rails-html-sanitizer (~> 1.1, >= 1.2.0)
-    activesupport (7.0.4)
+    activesupport (7.0.4.3)
       concurrent-ruby (~> 1.0, >= 1.0.2)
       i18n (>= 1.6, < 2)
       minitest (>= 5.1)
       tzinfo (~> 2.0)
     bcrypt_pbkdf (1.1.0)
     builder (3.2.4)
-    concurrent-ruby (1.1.10)
+    concurrent-ruby (1.2.2)
     crass (1.0.6)
     debug (1.7.1)
-      irb (>= 1.5.0)
-      reline (>= 0.3.1)
     dotenv (2.8.1)
     ed25519 (1.3.0)
     erubi (1.12.0)
     i18n (1.12.0)
       concurrent-ruby (~> 1.0)
-    io-console (0.6.0)
-    irb (1.6.2)
-      reline (>= 0.3.0)
     loofah (2.19.1)
       crass (~> 1.0.2)
       nokogiri (>= 1.5.9)
     method_source (1.0.0)
-    minitest (5.17.0)
+    minitest (5.18.0)
     mocha (2.0.2)
       ruby2_keywords (>= 0.0.5)
     net-scp (4.0.0)
       net-ssh (>= 2.6.5, < 8.0.0)
-    net-ssh (7.0.1)
-    nokogiri (1.14.0-arm64-darwin)
+    net-ssh (7.1.0)
+    nokogiri (1.14.2-arm64-darwin)
       racc (~> 1.4)
-    nokogiri (1.14.0-x86_64-darwin)
+    nokogiri (1.14.2-x86_64-darwin)
       racc (~> 1.4)
-    nokogiri (1.14.0-x86_64-linux)
+    nokogiri (1.14.2-x86_64-linux)
       racc (~> 1.4)
     racc (1.6.2)
-    rack (2.2.5)
-    rack-test (2.0.2)
+    rack (2.2.6.4)
+    rack-test (2.1.0)
       rack (>= 1.3)
     rails-dom-testing (2.0.3)
       activesupport (>= 4.2.0)
       nokogiri (>= 1.6)
-    rails-html-sanitizer (1.4.4)
+    rails-html-sanitizer (1.5.0)
       loofah (~> 2.19, >= 2.19.1)
-    railties (7.0.4)
-      actionpack (= 7.0.4)
-      activesupport (= 7.0.4)
+    railties (7.0.4.3)
+      actionpack (= 7.0.4.3)
+      activesupport (= 7.0.4.3)
       method_source
       rake (>= 12.2)
       thor (~> 1.0)
       zeitwerk (~> 2.5)
     rake (13.0.6)
-    reline (0.3.2)
-      io-console (~> 0.5)
     ruby2_keywords (0.0.5)
-    sshkit (1.21.3)
+    sshkit (1.21.4)
       net-scp (>= 1.1.2)
       net-ssh (>= 2.8.0)
     thor (1.2.1)
-    tzinfo (2.0.5)
+    tzinfo (2.0.6)
       concurrent-ruby (~> 1.0)
-    zeitwerk (2.6.6)
+    zeitwerk (2.6.7)
 
 PLATFORMS
-  arm64-darwin-20
-  arm64-darwin-21
-  arm64-darwin-22
-  x86_64-darwin-20
-  x86_64-darwin-21
-  x86_64-darwin-22
+  arm64-darwin
+  x86_64-darwin
   x86_64-linux
 
 DEPENDENCIES

From 14512fe40987a046c1e4c9f00301b7204b3a3c53 Mon Sep 17 00:00:00 2001
From: David Heinemeier Hansson <dhh@hey.com>
Date: Thu, 23 Mar 2023 12:10:56 +0100
Subject: [PATCH 13/19] Update README.md

---
 README.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/README.md b/README.md
index eaa17aea..4119a77d 100644
--- a/README.md
+++ b/README.md
@@ -4,7 +4,7 @@ MRSK deploys web apps anywhere from bare metal to cloud VMs using Docker with ze
 
 Watch the screencast: https://www.youtube.com/watch?v=LL1cV2FXZ5I
 
-Join us on Discord: https://discord.gg/DQETs3Pm
+Join us on Discord: https://discord.gg/YgHVT7GCXS
 
 ## Installation
 

From 9a909ba7ebbb7cca3503234c28ed3f17e413d7cb Mon Sep 17 00:00:00 2001
From: David Heinemeier Hansson <dhh@hey.com>
Date: Thu, 23 Mar 2023 14:06:15 +0100
Subject: [PATCH 14/19] config.traefik is already nil safe

---
 lib/mrsk/commands/traefik.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/mrsk/commands/traefik.rb b/lib/mrsk/commands/traefik.rb
index e09c527b..0b462fa0 100644
--- a/lib/mrsk/commands/traefik.rb
+++ b/lib/mrsk/commands/traefik.rb
@@ -56,7 +56,7 @@ class Mrsk::Commands::Traefik < Mrsk::Commands::Base
 
   private
     def docker_option_args
-      if args = config.raw_config.dig(:traefik, "options")
+      if args = config.traefik["options"]
         optionize args
       else
         []

From 8b755c6973fa71b78c56e7f44b6387e95191154a Mon Sep 17 00:00:00 2001
From: David Heinemeier Hansson <dhh@hey.com>
Date: Thu, 23 Mar 2023 14:24:34 +0100
Subject: [PATCH 15/19] Style

---
 lib/mrsk/commands/traefik.rb | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/lib/mrsk/commands/traefik.rb b/lib/mrsk/commands/traefik.rb
index 0b462fa0..ee86543f 100644
--- a/lib/mrsk/commands/traefik.rb
+++ b/lib/mrsk/commands/traefik.rb
@@ -10,7 +10,7 @@ class Mrsk::Commands::Traefik < Mrsk::Commands::Base
       "--log-opt", "max-size=#{MAX_LOG_SIZE}",
       "--publish", port,
       "--volume", "/var/run/docker.sock:/var/run/docker.sock",
-      *docker_option_args,
+      *docker_options_args,
       "traefik",
       "--providers.docker",
       "--log.level=DEBUG",
@@ -55,12 +55,8 @@ class Mrsk::Commands::Traefik < Mrsk::Commands::Base
   end
 
   private
-    def docker_option_args
-      if args = config.traefik["options"]
-        optionize args
-      else
-        []
-      end
+    def docker_options_args
+      optionize(config.traefik["options"] || {})
     end
 
     def cmd_option_args

From 951a71f38eb38f893289de0db9451e499fd88dc7 Mon Sep 17 00:00:00 2001
From: David Heinemeier Hansson <dhh@hey.com>
Date: Thu, 23 Mar 2023 14:26:12 +0100
Subject: [PATCH 16/19] Style

---
 lib/mrsk/utils.rb | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/lib/mrsk/utils.rb b/lib/mrsk/utils.rb
index 249e2ded..b7515c5b 100644
--- a/lib/mrsk/utils.rb
+++ b/lib/mrsk/utils.rb
@@ -25,11 +25,10 @@ module Mrsk::Utils
 
   # Returns a list of shell-dashed option arguments. If the value is true, it's treated like a value-less option.
   def optionize(args, with: nil)
-    flattened_args = flatten_args args
     options = if with
-      flattened_args.collect { |(key, value)| value == true ? "--#{key}" : "--#{key}#{with}#{escape_shell_value(value)}" }
+      flatten_args(args).collect { |(key, value)| value == true ? "--#{key}" : "--#{key}#{with}#{escape_shell_value(value)}" }
     else
-      flattened_args.collect { |(key, value)| [ "--#{key}", value == true ? nil : escape_shell_value(value) ] }
+      flatten_args(args).collect { |(key, value)| [ "--#{key}", value == true ? nil : escape_shell_value(value) ] }
     end
 
     options.flatten.compact
@@ -37,7 +36,7 @@ module Mrsk::Utils
 
   # Flattens a one-to-many structure into an array of two-element arrays each containing a key-value pair
   def flatten_args(args)
-    args.flat_map { |key, value| value.respond_to?('map') ? value.map { |entry| [key, entry] }: [[key, value]] }
+    args.flat_map { |key, value| value.try(:map) { |entry| [key, entry] } || [ [ key, value ] ] }
   end
 
   # Copied from SSHKit::Backend::Abstract#redact to be available inside Commands classes

From f73c526890430f1653e79a973119789bcb2f94d7 Mon Sep 17 00:00:00 2001
From: David Heinemeier Hansson <dhh@hey.com>
Date: Thu, 23 Mar 2023 14:46:41 +0100
Subject: [PATCH 17/19] Ask for access token

---
 lib/mrsk/cli/templates/deploy.yml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/mrsk/cli/templates/deploy.yml b/lib/mrsk/cli/templates/deploy.yml
index 55c53149..7d710632 100644
--- a/lib/mrsk/cli/templates/deploy.yml
+++ b/lib/mrsk/cli/templates/deploy.yml
@@ -13,6 +13,8 @@ registry:
   # Specify the registry server, if you're not using Docker Hub
   # server: registry.digitalocean.com / ghcr.io / ...
   username: my-user
+
+  # Always use an access token rather than real password when possible.
   password:
     - MRSK_REGISTRY_PASSWORD
 

From 1887a6518e02c0f9ef83250f4717b01e82d37536 Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremydaer@gmail.com>
Date: Wed, 22 Mar 2023 09:45:50 -0700
Subject: [PATCH 18/19] Commander needn't accumulate configuration

Commander had version/destination solely to incrementally accumulate CLI
options. Simpler to configure in one shot.

Clarifies responsibility and lets us introduce things like
`abbreviated_version` in one spot - Configuration.
---
 lib/mrsk/cli/app.rb                 |  2 +-
 lib/mrsk/cli/base.rb                | 17 +++++++-------
 lib/mrsk/cli/build.rb               |  2 +-
 lib/mrsk/cli/main.rb                |  2 +-
 lib/mrsk/commander.rb               | 36 +++++++++++++----------------
 lib/mrsk/configuration.rb           | 26 +++++++++++++++++----
 test/cli/main_test.rb               | 32 ++++++++++++++++++++++---
 test/commander_test.rb              | 14 +++--------
 test/configuration_test.rb          | 28 ++++++++++++++++------
 test/fixtures/deploy_with_roles.yml |  5 ++--
 10 files changed, 105 insertions(+), 59 deletions(-)

diff --git a/lib/mrsk/cli/app.rb b/lib/mrsk/cli/app.rb
index 796cd367..0898a509 100644
--- a/lib/mrsk/cli/app.rb
+++ b/lib/mrsk/cli/app.rb
@@ -37,7 +37,7 @@ class Mrsk::Cli::App < Mrsk::Cli::Base
   desc "start", "Start existing app container on servers"
   def start
     on(MRSK.hosts) do
-      execute *MRSK.auditor.record("Started app version #{MRSK.version}"), verbosity: :debug
+      execute *MRSK.auditor.record("Started app version #{MRSK.config.version}"), verbosity: :debug
       execute *MRSK.app.start, raise_on_non_zero_exit: false
     end
   end
diff --git a/lib/mrsk/cli/base.rb b/lib/mrsk/cli/base.rb
index c7a87964..4e0f2104 100644
--- a/lib/mrsk/cli/base.rb
+++ b/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
 
diff --git a/lib/mrsk/cli/build.rb b/lib/mrsk/cli/build.rb
index 3e8912d7..c9845e4f 100644
--- a/lib/mrsk/cli/build.rb
+++ b/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
diff --git a/lib/mrsk/cli/main.rb b/lib/mrsk/cli/main.rb
index 1524b49e..eb441d30 100644
--- a/lib/mrsk/cli/main.rb
+++ b/lib/mrsk/cli/main.rb
@@ -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
diff --git a/lib/mrsk/commander.rb b/lib/mrsk/commander.rb
index be2fd2e3..5aa24fc7 100644
--- a/lib/mrsk/commander.rb
+++ b/lib/mrsk/commander.rb
@@ -1,19 +1,26 @@
 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)
-    @config_file, @destination, @verbosity = config_file, destination, verbosity
+  def initialize
+    self.verbosity = :info
   end
 
+
   def config
-    @config ||= \
-      Mrsk::Configuration
-        .create_from(config_file, destination: destination, version: cascading_version)
-        .tap { |config| configure_sshkit_with(config) }
+    @config ||= Mrsk::Configuration.create_from(**@config_kwargs).tap do |config|
+      @config_kwargs = nil
+      configure_sshkit_with(config)
+    end
   end
 
+  def configure(**kwargs)
+    @config, @config_kwargs = nil, kwargs
+  end
+
+
   attr_accessor :specific_hosts
 
   def specific_primary!
@@ -90,26 +97,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 }
diff --git a/lib/mrsk/configuration.rb b/lib/mrsk/configuration.rb
index 1babea37..f95de700 100644
--- a/lib/mrsk/configuration.rb
+++ b/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")
-      raw_config = load_config_files(base_config_file, *destination_config_file(base_config_file, destination))
+    def create_from(config_file:, destination: nil, version: nil)
+      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
@@ -203,4 +210,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
diff --git a/test/cli/main_test.rb b/test/cli/main_test.rb
index 086067ff..151fa7b8 100644
--- a/test/cli/main_test.rb
+++ b/test/cli/main_test.rb
@@ -126,7 +126,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]
@@ -138,6 +138,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)
@@ -227,7 +253,7 @@ class CliMainTest < CliTestCase
   end
 
   private
-    def run_command(*command)
-      stdouted { Mrsk::Cli::Main.start([*command, "-c", "test/fixtures/deploy_with_accessories.yml"]) }
+    def run_command(*command, config_file: "deploy_with_accessories")
+      stdouted { Mrsk::Cli::Main.start([*command, "-c", "test/fixtures/#{config_file}.yml"]) }
     end
 end
diff --git a/test/commander_test.rb b/test/commander_test.rb
index b4ce24dc..278d9896 100644
--- a/test/commander_test.rb
+++ b/test/commander_test.rb
@@ -2,23 +2,15 @@ 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
     assert_equal Mrsk::Configuration, @mrsk.config.class
   end
 
-  test "commit hash as version" do
-    assert_equal `git rev-parse HEAD`.strip, @mrsk.config.version
-  end
-
-  test "commit hash as version but not in git" do
-    @mrsk.expects(:system).with("git rev-parse").returns(nil)
-    error = assert_raises(RuntimeError) { @mrsk.config }
-    assert_match /no git repository found/, error.message
-  end
-
   test "overwriting hosts" do
     assert_equal [ "1.1.1.1", "1.1.1.2", "1.1.1.3", "1.1.1.4" ], @mrsk.hosts
 
diff --git a/test/configuration_test.rb b/test/configuration_test.rb
index 5cd7856b..30c4b049 100644
--- a/test/configuration_test.rb
+++ b/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,7 +22,8 @@ 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
@@ -66,8 +68,20 @@ class ConfigurationTest < ActiveSupport::TestCase
   end
 
   test "version" do
-    assert_equal "missing", @config.version
-    assert_equal "123", Mrsk::Configuration.new(@deploy, version: "123").version
+    ENV.delete("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
@@ -158,17 +172,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 +190,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
 
diff --git a/test/fixtures/deploy_with_roles.yml b/test/fixtures/deploy_with_roles.yml
index 345c6301..0e405241 100644
--- a/test/fixtures/deploy_with_roles.yml
+++ b/test/fixtures/deploy_with_roles.yml
@@ -5,8 +5,9 @@ servers:
     - 1.1.1.1
     - 1.1.1.2
   workers:
-    - 1.1.1.3
-    - 1.1.1.4
+    hosts:
+      - 1.1.1.3
+      - 1.1.1.4
 env:
   REDIS_URL: redis://x/y
 registry:

From 035e4afff712abfe66acb01e71105eb61c728f3b Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremydaer@gmail.com>
Date: Wed, 22 Mar 2023 18:56:57 -0700
Subject: [PATCH 19/19] Validate that all roles have hosts

---
 lib/mrsk/configuration.rb      |  8 +++++-
 lib/mrsk/configuration/role.rb |  6 +++-
 test/configuration_test.rb     | 52 +++++++++++++++++++++++++++++-----
 3 files changed, 57 insertions(+), 9 deletions(-)

diff --git a/lib/mrsk/configuration.rb b/lib/mrsk/configuration.rb
index f95de700..074c7cf3 100644
--- a/lib/mrsk/configuration.rb
+++ b/lib/mrsk/configuration.rb
@@ -80,7 +80,7 @@ class Mrsk::Configuration
   end
 
   def primary_web_host
-    role(:web).hosts.first
+    role(:web).primary_host
   end
 
   def traefik_hosts
@@ -196,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
 
diff --git a/lib/mrsk/configuration/role.rb b/lib/mrsk/configuration/role.rb
index f3e20e9a..bfc42fbe 100644
--- a/lib/mrsk/configuration/role.rb
+++ b/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
 
diff --git a/test/configuration_test.rb b/test/configuration_test.rb
index 30c4b049..ee381449 100644
--- a/test/configuration_test.rb
+++ b/test/configuration_test.rb
@@ -26,14 +26,19 @@ class ConfigurationTest < ActiveSupport::TestCase
     ENV.delete("VERSION")
   end
 
-  test "ensure valid keys" do
-    assert_raise(ArgumentError) do
-      Mrsk::Configuration.new(@deploy.tap { _1.delete(:service) })
-      Mrsk::Configuration.new(@deploy.tap { _1.delete(:image) })
-      Mrsk::Configuration.new(@deploy.tap { _1.delete(:registry) })
+  %i[ service image registry ].each do |key|
+    test "#{key} config required" do
+      assert_raise(ArgumentError) do
+        Mrsk::Configuration.new @deploy.tap { _1.delete key }
+      end
+    end
+  end
 
-      Mrsk::Configuration.new(@deploy.tap { _1[:registry].delete("username") })
-      Mrsk::Configuration.new(@deploy.tap { _1[:registry].delete("password") })
+  %w[ username password ].each do |key|
+    test "registry #{key} required" do
+      assert_raise(ArgumentError) do
+        Mrsk::Configuration.new @deploy.tap { _1[:registry].delete key }
+      end
     end
   end
 
@@ -148,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