Add docker-setup hook

[tsvallender]

This allows the user to make any necessary configuration changes to Docker before setting up any containers, allowing those configuration changes to take effect from the outset.

My primary motivation for adding this was wanting all the containers to be on the same Docker network. This change means the docker-setup hook script can ensure the network exists before deployment rather than needing to do so manually. This should be more broadly useful for any general Docker configuration that may be necessary by the user, however. The new hook point is necessary so it can be done after we ensure Docker is installed but before deploying any containers.

I’ve tested this manually but don’t fully understand the way hooks are being tested so would appreciate some assistance getting the new test to pass.

[djmb]

@tsvallender - I'm not sure why the test is failing, at first glance it looks like it should work.

If you move the calling the hook into kamal:cli:server:bootstrap then we'll need to move the test into test/cli/server_test.rb anyway. Let's do that first and then we can debug it from there.

[tsvallender]

@tsvallender - I'm not sure why the test is failing, at first glance it looks like it should work.

If you move the calling the hook into kamal:cli:server:bootstrap then we'll need to move the test into test/cli/server_test.rb anyway. Let's do that first and then we can debug it from there.

Thanks for the feedback. I’ve made the changes you suggested and done another manual test. Still getting the same error in the test suite though so any thoughts on that appreciated! I can see the Running the docker-setup hook... text included in the output when running things, so I presume it must be something around the test setup but I don’t know what 🤔

[tsvallender]

Just done a bit more investigation and it seems like in the test environment the hook isn’t running because Running the docker-setup hook...KAMAL.hook.hook_exists?(hook) is returning false 🤔 Thought for a moment it was because the test hook wasn’t executable but changing that doesn’t seem to have fixed anything…

[tsvallender]

Hm, made a bit more progress. Realised I needed to stub hook_exists?, which gets us past that error. This is the error we have now, which is thrown regardless of whether or not there is an assertion for the hook being run.

Failure:
CliServerTest#test_bootstrap_install_as_root_user [/home/tsv/code/kamal/lib/kamal/cli/base.rb:149]:
unexpected invocation: #<AnyInstance:SSHKit::Backend::Abstract>.execute(".kamal/hooks/docker-setup", {:env => {"KAMAL_RECORDED_AT" => "2023-11-19T11:00:39Z", "KAMAL_PERFORMER" => "tsv", "KAMAL
_VERSION" => "999", "KAMAL_SERVICE_VERSION" => "app@999", "KAMAL_HOSTS" => "1.1.1.1,1.1.1.2,1.1.1.3,1.1.1.4", "KAMAL_COMMAND" => "server", "KAMAL_SUBCOMMAND" => "bootstrap"}})
satisfied expectations:
- expected at least once, invoked 4 times: #<AnyInstance:SSHKit::Backend::Abstract>.execute(:mkdir, "-p", ".kamal")
- expected at least once, invoked 4 times: #<AnyInstance:SSHKit::Backend::Abstract>.execute(:curl, "-fsSL", "https://get.docker.com", "|", :sh)
- expected at least once, invoked 4 times: #<AnyInstance:SSHKit::Backend::Abstract>.execute("[ \"${EUID:-$(id -u)}\" -eq 0 ] || command -v sudo >/dev/null || command -v su >/dev/null", :raise_
on_non_zero_exit => false)
- expected at least once, invoked 4 times: #<AnyInstance:SSHKit::Backend::Abstract>.execute(:docker, "-v", :raise_on_non_zero_exit => false)
- allowed any number of times, invoked once: #<AnyInstance:Kamal::Commands::Hook>.hook_exists?(any_parameters)

[99linesofcode]

I'm a fan. If this is accepted please make sure to update the documention on hooks accordingly.

[tsvallender]

Just came back to this and managed to get the tests passing 🙌

Hopefully it can progress now—if there’s anything I can do to make that happen let me know.

[fredrikburmester]

The kamal remove command does not remove the network created with this command. Is this the expected behaviour?

[tsvallender]

The kamal remove command does not remove the network created with this command. Is this the expected behaviour?

Good question. Debatable, I suppose. I guess adding another hook at the point of removal might be the best way to do this. I can take a look at adding this in soon.