Add a docker based instance of `Remote`.

jepsen-io / jepsen

A framework for distributed systems verification, with fault injection

6.69k stars 710 forks source link

Add a docker based instance of `Remote`. #460

Closed stevana closed 4 years ago

stevana commented 4 years ago

This is useful for avoiding the jepsen-control jumpbox container in the docker-compose based workflow. Instead of docker execing into the jepsen-control container and using ssh from there to do stuff to the nodes, this change lets us docker exec/cp directly into the nodes from our local machine -- thus simplifying the developer workflow.

stevana commented 4 years ago

Few questions:

Shall I wrap all operations in when-not dummy, or is that a SSH only thing?
~Can we do without undo-command-changes?~ Yes
Can we do something better than passing in nodes to docker and using .indexOf to figure out the container name?

stevana commented 4 years ago

Build seems broken because Could not find artifact elle:elle:jar:0.1.1-SNAPSHOT in clojars (https://repo.clojars.org/).

aphyr commented 4 years ago

Ah, I just cut that snapshot yesterday. It's on Clojars now, sorry!

aphyr commented 4 years ago

Shall I wrap all operations in when-not dummy, or is that a SSH only thing?

I think... yeah, you should probably respect dummy for now. It might be the case that we can get rid of dummy altogether and replace it with a no-op implementation of the Remote protocol--it was invented for cases where people wanted to run tests without any SSH connections at all; e.g. testing third-party APIs.

stevana commented 4 years ago

Moved to separate namespace, put a big fat warning saying this isn't the recommend way of running Jepsen
Stole @keynslug's code to improve escaping and error handling (I hope that's ok with you @keynslug?)
Removed the need to pass in nodes and the ugly .indexOf trick to figure out the host name, inspired by @keynslug but ultimately a different approach
I didn't respect dummy in the end, it seemed enough that the use case you mention "it was invented for cases where people wanted to run tests without any SSH connections at all; e.g. testing third-party APIs." is already covered and it seems unnecessary to support it in two different ways (i.e. via DockerRemote also). I'm happy to try to have a go at the noop-remote in a separate PR though.

Thoughts?

aphyr commented 4 years ago

Nice work! Yeah, this addresses all my concerns, and I don't see any obvious pitfalls. I also agree, on further thought, that dummy+ssh works fine; no need to support it here. Let's go! :)