Make docker engine port configurable

Introduced -engine-port flag in create subcommand
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 docker-archive-public#3361

Signed-off-by: Wouter Dullaert <[email protected]>
(cherry picked from commit edad5f7)

Author: wdullaer

commands/create.go

@@ -77,6 +77,12 @@ var (
 			Usage: "Specify environment variables to set in the engine",
 			Value: &cli.StringSlice{},
 		},
+		cli.IntFlag{
+			Name:   "engine-port",
+			Usage:  "Specify the port number that the engine will listen on",
+			Value:  engine.DefaultPort,
+			EnvVar: "ENGINE_PORT_NUMBER",
+		},
 		cli.BoolFlag{
 			Name:  "swarm",
 			Usage: "Configure Machine to join a Swarm cluster",
@@ -157,6 +163,7 @@ func cmdCreateInner(c CommandLine, api libmachine.API) error {
 	rawDriver, err := json.Marshal(&drivers.BaseDriver{
 		MachineName: name,
 		StorePath:   c.GlobalString("storage-path"),
+		PortNumber:  c.Int("engine-port"),
 	})
 	if err != nil {
 		return fmt.Errorf("Error attempting to marshal bare driver data: %s", err)

commands/ls_test.go

@@ -317,6 +317,7 @@ func TestGetHostListItems(t *testing.T) {
 			Driver: &fakedriver.Driver{
 				MockState: state.Running,
 				MockIP:    "active.host.com",
+				MockPort:  2376,
 			},
 		},
 		{

commands/url_test.go

@@ -42,6 +42,7 @@ func TestCmdURL(t *testing.T) {
 				Driver: &fakedriver.Driver{
 					MockState: state.Running,
 					MockIP:    "120.0.0.1",
+					MockPort:  2376,
 				},
 			},
 		},

drivers/amazonec2/amazonec2.go

@@ -50,7 +50,6 @@ const (
 )
 
 var (
-	dockerPort                           = 2376
 	swarmPort                            = 3376
 	errorNoPrivateSSHKey                 = errors.New("using --amazonec2-keypair-name also requires --amazonec2-ssh-keypath")
 	errorMissingCredentials              = errors.New("amazonec2 driver requires AWS credentials configured with the --amazonec2-access-key and --amazonec2-secret-key options, environment variables, ~/.aws/credentials, or an instance role")
@@ -759,7 +758,7 @@ func (d *Driver) GetURL() (string, error) {
 		return "", nil
 	}
 
-	return fmt.Sprintf("tcp://%s", net.JoinHostPort(ip, strconv.Itoa(dockerPort))), nil
+	return fmt.Sprintf("tcp://%s", net.JoinHostPort(ip, strconv.Itoa(d.GetPort()))), nil
 }
 
 func (d *Driver) GetIP() (string, error) {
@@ -1128,11 +1127,11 @@ func (d *Driver) configureSecurityGroupPermissions(group *ec2.SecurityGroup) ([]
 		})
 	}
 
-	if !hasPorts[fmt.Sprintf("%d/tcp", dockerPort)] {
+	if !hasPorts[fmt.Sprintf("%d/tcp", d.GetPort())] {
 		perms = append(perms, &ec2.IpPermission{
 			IpProtocol: aws.String("tcp"),
-			FromPort:   aws.Int64(int64(dockerPort)),
-			ToPort:     aws.Int64(int64(dockerPort)),
+			FromPort:   aws.Int64(int64(d.GetPort())),
+			ToPort:     aws.Int64(int64(d.GetPort())),
 			IpRanges:   []*ec2.IpRange{{CidrIp: aws.String(ipRange)}},
 		})
 	}

drivers/amazonec2/amazonec2_test.go

@@ -19,7 +19,7 @@ import (
 
 const (
 	testSSHPort    = int64(22)
-	testDockerPort = int64(2376)
+	testDockerPort = int64(12345)
 	testSwarmPort  = int64(3376)
 )
 
@@ -32,7 +32,7 @@ var (
 )
 
 func TestConfigureSecurityGroupPermissionsEmpty(t *testing.T) {
-	driver := NewTestDriver()
+	driver := NewTestDriver(int(testDockerPort))
 
 	perms, err := driver.configureSecurityGroupPermissions(securityGroup)
 
@@ -41,7 +41,7 @@ func TestConfigureSecurityGroupPermissionsEmpty(t *testing.T) {
 }
 
 func TestConfigureSecurityGroupPermissionsSshOnly(t *testing.T) {
-	driver := NewTestDriver()
+	driver := NewTestDriver(int(testDockerPort))
 	group := securityGroup
 	group.IpPermissions = []*ec2.IpPermission{
 		{
@@ -59,7 +59,7 @@ func TestConfigureSecurityGroupPermissionsSshOnly(t *testing.T) {
 }
 
 func TestConfigureSecurityGroupPermissionsDockerOnly(t *testing.T) {
-	driver := NewTestDriver()
+	driver := NewTestDriver(int(testDockerPort))
 	group := securityGroup
 	group.IpPermissions = []*ec2.IpPermission{
 		{
@@ -77,7 +77,7 @@ func TestConfigureSecurityGroupPermissionsDockerOnly(t *testing.T) {
 }
 
 func TestConfigureSecurityGroupPermissionsDockerAndSsh(t *testing.T) {
-	driver := NewTestDriver()
+	driver := NewTestDriver(int(testDockerPort))
 	group := securityGroup
 	group.IpPermissions = []*ec2.IpPermission{
 		{
@@ -99,7 +99,7 @@ func TestConfigureSecurityGroupPermissionsDockerAndSsh(t *testing.T) {
 }
 
 func TestConfigureSecurityGroupPermissionsOpenPorts(t *testing.T) {
-	driver := NewTestDriver()
+	driver := NewTestDriver(int(testDockerPort))
 	driver.OpenPorts = []string{"8888/tcp", "8080/udp", "9090"}
 	perms, err := driver.configureSecurityGroupPermissions(&ec2.SecurityGroup{})
 
@@ -114,7 +114,7 @@ func TestConfigureSecurityGroupPermissionsOpenPorts(t *testing.T) {
 }
 
 func TestConfigureSecurityGroupPermissionsOpenPortsSkipExisting(t *testing.T) {
-	driver := NewTestDriver()
+	driver := NewTestDriver(int(testDockerPort))
 	group := securityGroup
 	group.IpPermissions = []*ec2.IpPermission{
 		{
@@ -137,7 +137,7 @@ func TestConfigureSecurityGroupPermissionsOpenPortsSkipExisting(t *testing.T) {
 }
 
 func TestConfigureSecurityGroupPermissionsInvalidOpenPorts(t *testing.T) {
-	driver := NewTestDriver()
+	driver := NewTestDriver(int(testDockerPort))
 	driver.OpenPorts = []string{"2222/tcp", "abc1"}
 	perms, err := driver.configureSecurityGroupPermissions(&ec2.SecurityGroup{})
 
@@ -146,7 +146,7 @@ func TestConfigureSecurityGroupPermissionsInvalidOpenPorts(t *testing.T) {
 }
 
 func TestConfigureSecurityGroupPermissionsWithSwarm(t *testing.T) {
-	driver := NewTestDriver()
+	driver := NewTestDriver(int(testDockerPort))
 	driver.SwarmMaster = true
 	group := securityGroup
 	group.IpPermissions = []*ec2.IpPermission{
@@ -219,7 +219,7 @@ func TestDefaultVPCIsMissing(t *testing.T) {
 }
 
 func TestGetRegionZoneForDefaultEndpoint(t *testing.T) {
-	driver := NewCustomTestDriver(&fakeEC2WithLogin{})
+	driver := NewCustomTestDriver(int(testDockerPort), &fakeEC2WithLogin{})
 	driver.awsCredentialsFactory = NewValidAwsCredentials
 	options := &commandstest.FakeFlagger{
 		Data: map[string]interface{}{
@@ -238,7 +238,7 @@ func TestGetRegionZoneForDefaultEndpoint(t *testing.T) {
 }
 
 func TestGetRegionZoneForCustomEndpoint(t *testing.T) {
-	driver := NewCustomTestDriver(&fakeEC2WithLogin{})
+	driver := NewCustomTestDriver(int(testDockerPort), &fakeEC2WithLogin{})
 	driver.awsCredentialsFactory = NewValidAwsCredentials
 	options := &commandstest.FakeFlagger{
 		Data: map[string]interface{}{
@@ -272,7 +272,7 @@ func TestDescribeAccountAttributeFails(t *testing.T) {
 }
 
 func TestAwsCredentialsAreRequired(t *testing.T) {
-	driver := NewTestDriver()
+	driver := NewTestDriver(int(testDockerPort))
 	driver.awsCredentialsFactory = NewErrorAwsCredentials
 
 	options := &commandstest.FakeFlagger{
@@ -288,7 +288,7 @@ func TestAwsCredentialsAreRequired(t *testing.T) {
 }
 
 func TestValidAwsCredentialsAreAccepted(t *testing.T) {
-	driver := NewCustomTestDriver(&fakeEC2WithLogin{})
+	driver := NewCustomTestDriver(int(testDockerPort), &fakeEC2WithLogin{})
 	driver.awsCredentialsFactory = NewValidAwsCredentials
 	options := &commandstest.FakeFlagger{
 		Data: map[string]interface{}{
@@ -303,7 +303,7 @@ func TestValidAwsCredentialsAreAccepted(t *testing.T) {
 }
 
 func TestEndpointIsMandatoryWhenSSLDisabled(t *testing.T) {
-	driver := NewTestDriver()
+	driver := NewTestDriver(int(testDockerPort))
 	driver.awsCredentialsFactory = NewValidAwsCredentials
 	options := &commandstest.FakeFlagger{
 		Data: map[string]interface{}{
@@ -402,7 +402,7 @@ func ipPermission(port int64) *ec2.IpPermission {
 func TestConfigureSecurityGroupsEmpty(t *testing.T) {
 	recorder := fakeEC2SecurityGroupTestRecorder{}
 
-	driver := NewCustomTestDriver(&recorder)
+	driver := NewCustomTestDriver(int(testDockerPort), &recorder)
 	err := driver.configureSecurityGroups([]string{})
 
 	assert.Nil(t, err)
@@ -457,7 +457,7 @@ func TestConfigureSecurityGroupsMixed(t *testing.T) {
 	}).Return(
 		&ec2.AuthorizeSecurityGroupIngressOutput{}, nil)
 
-	driver := NewCustomTestDriver(&recorder)
+	driver := NewCustomTestDriver(int(testDockerPort), &recorder)
 	err := driver.configureSecurityGroups(groups)
 
 	assert.Nil(t, err)
@@ -472,15 +472,15 @@ func TestConfigureSecurityGroupsErrLookupExist(t *testing.T) {
 	recorder.On("DescribeSecurityGroups", mock.MatchedBy(matchGroupLookup(groups))).Return(
 		nil, lookupExistErr)
 
-	driver := NewCustomTestDriver(&recorder)
+	driver := NewCustomTestDriver(int(testDockerPort), &recorder)
 	err := driver.configureSecurityGroups(groups)
 
 	assert.Exactly(t, lookupExistErr, err)
 	recorder.AssertExpectations(t)
 }
 
 func TestBase64UserDataIsEmptyIfNoFileProvided(t *testing.T) {
-	driver := NewTestDriver()
+	driver := NewTestDriver(int(testDockerPort))
 
 	userdata, err := driver.Base64UserData()
 
@@ -495,7 +495,7 @@ func TestBase64UserDataGeneratesErrorIfFileNotFound(t *testing.T) {
 	defer os.RemoveAll(dir)
 	userdata_path := filepath.Join(dir, "does-not-exist.yml")
 
-	driver := NewTestDriver()
+	driver := NewTestDriver(int(testDockerPort))
 	driver.UserDataFile = userdata_path
 
 	_, ud_err := driver.Base64UserData()
@@ -516,7 +516,7 @@ func TestBase64UserDataIsCorrectWhenFileProvided(t *testing.T) {
 	err = ioutil.WriteFile(userdata_path, content, 0666)
 	assert.NoError(t, err, "Unable to create temporary userdata file.")
 
-	driver := NewTestDriver()
+	driver := NewTestDriver(int(testDockerPort))
 	driver.UserDataFile = userdata_path
 
 	userdata, ud_err := driver.Base64UserData()

drivers/amazonec2/stub_test.go

@@ -132,16 +132,18 @@ func (f *fakeEC2SecurityGroupTestRecorder) AuthorizeSecurityGroupIngress(input *
 	return value, err
 }
 
-func NewTestDriver() *Driver {
+func NewTestDriver(dockerPort int) *Driver {
 	driver := NewDriver("machineFoo", "path")
+	driver.PortNumber = dockerPort
 	driver.clientFactory = func() Ec2Client {
 		return &fakeEC2{}
 	}
 	return driver
 }
 
-func NewCustomTestDriver(ec2Client Ec2Client) *Driver {
+func NewCustomTestDriver(dockerPort int, ec2Client Ec2Client) *Driver {
 	driver := NewDriver("machineFoo", "path")
+	driver.PortNumber = dockerPort
 	driver.clientFactory = func() Ec2Client {
 		return ec2Client
 	}

drivers/azure/azure.go

@@ -8,6 +8,7 @@ import (
 	"net"
 	"net/url"
 	"os"
+	"strconv"
 
 	"github.com/docker/machine/drivers/azure/azureutil"
 	"github.com/docker/machine/libmachine/drivers"
@@ -24,7 +25,6 @@ const (
 	defaultAzureSize            = "Standard_A2"
 	defaultAzureLocation        = "westus"
 	defaultSSHUser              = "docker-user" // 'root' not allowed on Azure
-	defaultDockerPort           = 2376
 	defaultAzureImage           = "canonical:UbuntuServer:16.04.0-LTS:latest"
 	defaultAzureVNet            = "docker-machine-vnet"
 	defaultAzureSubnet          = "docker-machine"
@@ -74,7 +74,6 @@ type Driver struct {
 	SubscriptionID string
 	ResourceGroup  string
 
-	DockerPort      int
 	Location        string
 	Size            string
 	Image           string
@@ -139,12 +138,6 @@ func (d *Driver) GetCreateFlags() []mcnflag.Flag {
 			EnvVar: "AZURE_SSH_USER",
 			Value:  defaultSSHUser,
 		},
-		mcnflag.IntFlag{
-			Name:   flAzureDockerPort,
-			Usage:  "Port number for Docker engine",
-			EnvVar: "AZURE_DOCKER_PORT",
-			Value:  defaultDockerPort,
-		},
 		mcnflag.StringFlag{
 			Name:   flAzureLocation,
 			Usage:  "Azure region to create the virtual machine",
@@ -233,6 +226,12 @@ func (d *Driver) GetCreateFlags() []mcnflag.Flag {
 			Usage:  "Azure Service Principal Account password (optional, browser auth is used if not specified)",
 			EnvVar: "AZURE_CLIENT_SECRET",
 		},
+		// DEPRECATED: remove in a future version
+		mcnflag.IntFlag{
+			Name:   flAzureDockerPort,
+			Usage:  "Port number for Docker engine",
+			EnvVar: "AZURE_DOCKER_PORT",
+		},
 	}
 }
 
@@ -273,7 +272,6 @@ func (d *Driver) SetConfigFromFlags(fl drivers.DriverOptions) error {
 	d.UsePrivateIP = fl.Bool(flAzureUsePrivateIP)
 	d.NoPublicIP = fl.Bool(flAzureNoPublicIP)
 	d.StaticPublicIP = fl.Bool(flAzureStaticPublicIP)
-	d.DockerPort = fl.Int(flAzureDockerPort)
 	d.DNSLabel = fl.String(flAzureDNSLabel)
 	d.CustomDataFile = fl.String(flAzureCustomData)
 
@@ -284,6 +282,10 @@ func (d *Driver) SetConfigFromFlags(fl drivers.DriverOptions) error {
 	d.BaseDriver.SSHPort = sshPort
 	d.SetSwarmConfigFromFlags(fl)
 
+	if fl.Int(flAzureDockerPort) != 0 {
+		return fmt.Errorf("-%s has been deprecated in favor of: -engine-port", flAzureDockerPort)
+	}
+
 	log.Debug("Set configuration from flags.")
 	return nil
 }
@@ -473,7 +475,7 @@ func (d *Driver) GetURL() (string, error) {
 	}
 	u := (&url.URL{
 		Scheme: "tcp",
-		Host:   net.JoinHostPort(ip, fmt.Sprintf("%d", d.DockerPort)),
+		Host:   net.JoinHostPort(ip, strconv.Itoa(d.GetPort())),
 	}).String()
 	log.Debugf("Machine URL is resolved to: %s", u)
 	return u, nil

drivers/azure/azure_test.go

@@ -0,0 +1,29 @@
+package azure
+
+import (
+	"testing"
+
+	"github.com/docker/machine/libmachine/drivers"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGenericDockerPortDeprecationError(t *testing.T) {
+	driver := NewDriver("default", "path")
+
+	checkFlags := &drivers.CheckDriverOptions{
+		FlagsValues: map[string]interface{}{
+			"azure-subscription-id": "abcdef",
+			"azure-docker-port":     12345,
+		},
+		CreateFlags: driver.GetCreateFlags(),
+	}
+
+	err := driver.SetConfigFromFlags(checkFlags)
+
+	assert.EqualError(
+		t,
+		err,
+		"-azure-docker-port has been deprecated in favor of: -engine-port",
+		"SetConfigFromFlags should throw an error when generic-docker-port is set",
+	)
+}

drivers/azure/util.go

@@ -109,12 +109,12 @@ func (d *Driver) getSecurityRules(extraPorts []string) (*[]network.SecurityRule,
 		}
 	}
 
-	log.Debugf("Docker port is configured as %d", d.DockerPort)
+	log.Debugf("Docker port is configured as %d", d.GetPort())
 
 	// Base ports to be opened for any machine
 	rl := []network.SecurityRule{
 		mkRule(100, "SSHAllowAny", "Allow ssh from public Internet", "*", fmt.Sprintf("%d", d.BaseDriver.SSHPort), network.TCP),
-		mkRule(300, "DockerAllowAny", "Allow docker engine access (TLS-protected)", "*", fmt.Sprintf("%d", d.DockerPort), network.TCP),
+		mkRule(300, "DockerAllowAny", "Allow docker engine access (TLS-protected)", "*", fmt.Sprintf("%d", d.GetPort()), network.TCP),
 	}
 
 	// Open swarm port if configured