adop-docker-compose icon indicating copy to clipboard operation
adop-docker-compose copied to clipboard

Added support for docker4mac

Open mcroker opened this issue 8 years ago • 9 comments

Added support for docker4mac (which doesn't use docker-machine)... invoked by quickstart -t mac.

mcroker avatar Sep 29 '16 16:09 mcroker

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?

nickdgriffin avatar Sep 30 '16 09:09 nickdgriffin

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:

./quickstart.sh -t local -m adop_local
./quickstart.sh -t native
./quickstart.sh -t aws

What do you think?

anton-kasperovich avatar Sep 30 '16 10:09 anton-kasperovich

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

mcroker avatar Sep 30 '16 15:09 mcroker

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?

nickdgriffin avatar Oct 03 '16 09:10 nickdgriffin

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.

mcroker avatar Oct 03 '16 09:10 mcroker

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.

nickdgriffin avatar Oct 04 '16 19:10 nickdgriffin

'-t localhost' perhaps... but otherwise lgtm...

mcroker avatar Oct 04 '16 20:10 mcroker

"-t localhost" is more logical.

nickdgriffin avatar Oct 05 '16 13:10 nickdgriffin

is this still an issue? can it be closed?

luismsousa avatar Oct 11 '18 14:10 luismsousa