Made docker engine port configurable

[wdullaer]

Extended drivers interface with a GetPort() method
Updated all drivers with the new interface
Updated provisioning to use GetPort() rather than parse GetURL() for the port
Made driver options for setting the engine port emit a useful error message
Fixes #3361

I think the behaviour of emitting a useful error for the driver flags that are replaced by -engine-port is a good compromise between being helpful without overcomplicating the code with backwards compatibility
Theoretically you could argue that the engine port should go in the engine options, rather than the driver options, but the way the code is structured this is very unwieldy. (especially because drivers like the aws one will need to tweak firewalls / security groups to make everything work)

As a side remark: the build and tests don't work if you have spaces in your source path somewhere.
I haven't been able to completely fix this, but if I do I'll open a separate PR

[afbjorklund]

@wdullaer : this sounds like a good idea! (making the port not hard-coded / configurable by drivers)

I needed this for KVM (qemu), but ended up making my own patch since I didn't found this one in time...

afbjorklund@824a68c

Will see if it is possible to merge these two somehow, as I'm using it for "user" networking with KVM.

---

https://www.linux-kvm.org/page/Networking#User_Networking

It is not perfect either way, since I have one port on the VM "outside" and one port on the VM "inside":

hostfwd=tcp::33761-:22,hostfwd=tcp::35905-:2376

For now, I just made it always return 2376 for when it's checking inside and after that return the real port

[wdullaer]

It's been half a year since I worked on this code, so it's not exactly fully in my head right now, but I had a quick look at your code and the diff doesn't seem to be that large.

I covered a lot more drivers than the commit you referred to, but I did seem to have missed the reference to the defaultPort in the boot2docker provisioner.

I see that your GetPort can also return an error. I don't really understand how this can fail: it's either the default value or some value the user has set, but it's not an operation that can actually fail right?

[afbjorklund]

About the error code, you are probably right - there was some quick copy/paste involved :-)

Happy to help with the merge, basically I just wanted to override the GetPort() in the driver.

        if d.Network == "user" {
                d.SSHPort, err = getAvailableTCPPort()
                if err != nil {
                        return err
                }
                d.EnginePort, err = getAvailableTCPPort()
                if err != nil {
                        return err
                }
        }

func (d *Driver) GetIP() (string, error) {
        if d.Network == "user" {
                return "127.0.0.1", nil
        }
        return d.NetworkAddress, nil
}

func (d *Driver) GetPort() (int, error) {
        return d.EnginePort, nil
}

It's not like there ever is any error... (nil)

https://wiki.qemu.org/Documentation/Networking#User_Networking_.28SLIRP.29

[wdullaer]

As far as I can tell, compared to your branch, I just missed the boot2docker provisioner, which I can add when I find an hour somewhere this week.

How do you propose to do the merge? I fix this branch up and you merge it into yours? We wait until this one gets merged by the maintainers?
You just copy my changes in your branch?

[afbjorklund]

@wdullaer : If you fix your branch (for boot2docker), then I can just use that instead of mine

Have been working on the driver instead. It's not the end of the world for it without GetPort(),
just that machine will try to look for it with the port from the URL instead of the actual port:

This machine has been allocated an IP address, but Docker Machine could not
reach it successfully.

SSH for the machine should still work, but connecting to exposed ports, such as
the Docker daemon port (usually <ip>:2376), may not work properly.

You may need to add the route manually, or use another related workaround.

This could be due to a VPN, proxy, or host file configuration issue.

You also might want to clear any VirtualBox host only interfaces you are not using.
Checking connection to Docker...
Docker is up and running!

Of course, it would look better with our additions and actually testing the right thing (port)...
So I will wait for your new updated Pull Request, and then just rebase my stuff on top of that.

[GordonTheTurtle]

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "engine-port" git@github.com:wdullaer/machine.git somewhere
$ cd somewhere
$ git rebase -i HEAD~842354159728
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

[wdullaer]

I've updated the PR with the fixed boot2docker provisioner.
Let me know if there are any other issues, or if there's anything else you want me to take a look at.

(I realized that the docs should also be updated to reflect the new --engine-port flag, but they're not maintained in this repository)

[afbjorklund]

@wdullaer : tried it out now, works fine. I had to rewrite my plugin, but that was for the best.
i.e. it was only providing the temporary URL before, but now it has to provide the port too...
(afbjorklund/docker-machine-driver-qemu@d09c88c)

That change was for the better, anyway. Only comment remaining is that you forgot to put the
mandatory newline between the git commit "subject" and "description" (smushing it together)
Hope that this get merged, so that we can avoid that silly netstat "warning" (quoted above).

[wdullaer]

That's great news! I'm happy this was useful for you.

If the biggest concern is about the formatting of the commit message, I consider that a succes.
I'll update it to comply with the rules shortly.

I also hope it gets merged at some point, but I have no idea who to talk to to move this forward to be honest