-
Notifications
You must be signed in to change notification settings - Fork 573
Added support for docker4mac #148
base: master
Are you sure you want to change the base?
Conversation
Hi Martin, The current compose init already supports not using docker-machine, that's what PROXY_IP was put in for. Running on a Mac is the same as running locally I expect, which is what Travis does (as it can't use Machine with VirtualBox). Also, the intention of quickstart.sh was meant to be just for the case of using docker-machine in front to provision a machine. Natively I'd expect just the CLI to be used. Are there other considerations needed for running on docker4mac? Also, judging by the fact it hasn't merged cleanly I think perhaps you need to refresh your branch from master? |
Hi Martin & Nick, I assume we can improve this PR a bit more, to support both Mac & Windows at the same time, as starting from Windows 10 it's also possible to use init script without Docker Machine, Docker Native etc... I'm thinking that we can make QuickStart works in a two cases like with/without machine:
What do you think? |
Anton, I agree completely and actually think '-t local' should possible be changed to '-t dockermachine' (but left it for backward compatibility). '-t native makes sense' Nick, Ok - so does that mean my change to cmd/compose can be ignored and instead implemented through a further change to use PROXY_IP ? (perhaps you do do this as you merge ;) )? Martin |
PROXY_IP was put in for the case where the stack would be accessed from a custom IP/domain (such as behind an SSL-enabled ELB), and the fall through in case Docker Machine wasn't being used so it defaults to localhost was put in place for Travis (https://github.com/Accenture/adop-docker-compose/blob/master/.travis.yml#L22). I thought the order of precedence should be custom domain > Docker Machine > local Engine. What was the intended outcome of your change that isn't currently covered by the logic? Is there a gap here when working locally that I'm missing? |
If PROXY_IP is not set the cmd/compose script assumes use of docker-machine. It fails with error if a docker-machine is specified that doesn't exist (or no machine is set and the default machine doesn't exist). My change was to override this behavior where no machine is specified (i.e. remove the assumption of a default docker-machine. The suggestion I made above was to leave cmd/compose unmodified - and have quickstart specify IP_PROXY as localhost where '-t native' is specified in the options. |
It wasn't expected that a machine name would be specified that was invalid so that's why the logic doesn't handle that in a useful way other than just bottling out - it assumes a mistake or issue, rather than falling through to localhost. If the -m flag isn't provided then it should fall through to defaulting to localhost as that's how it works in Travis. I think having "-t native" explicitly setting "-i localhost" is a good way to feed this in and make it clear to anyone who looks into the quickstart what is expected, instead of having to dive into "cmd/compose" which is a bit more complex. |
'-t localhost' perhaps... but otherwise lgtm... |
"-t localhost" is more logical. |
is this still an issue? can it be closed? |
Added support for docker4mac (which doesn't use docker-machine)... invoked by quickstart -t mac.