jepsen-io / jepsen

A framework for distributed systems verification, with fault injection
6.78k stars 715 forks source link

docker: export JEPSEN_ROOT again #466

Closed vjuranek closed 4 years ago

vjuranek commented 4 years ago

Export JEPSEN_ROOT again as it's required by docker-compose.dev.yml. Fixes GH #465

aphyr commented 4 years ago

@stevana, this undoes some changes from your recent patches--does this look right to you? I'm happy to merge, just wanna make sure this won't break something else.

stevana commented 4 years ago

Looks good to me.

I believe I didn't notice this problem when testing because I always used to set JEPSEN_ROOT explicitly, my bad.

I don't think this will break something else, because 1) it reverts back to how it used to be before my patch, 2) the only place this environment variable is used is in docker-compose.dev.yml.

An alternative fix would be to explicitly pass the environment variable to the children processes that need it, e.g. JEPSEN_ROOT="${JEPSEN_ROOT}" docker-compose -f docker-compose.yml ${COMPOSE} ${DEV} build, as this is what export does. I believe this would be more explicit and hence safe, but because of 2) I think as is done in this PR is fine.

aphyr commented 4 years ago

Righto. I'll merge this, and if y'all want to refactor later that's fine!