Merge pull request #686 from ehazlett/ec2-fix-swarm-port-auth

ec2: do not try to auth swarm port if exists

drivers/amazonec2/amazonec2.go

@@ -29,7 +29,11 @@ const (
 	ipRange                  = "0.0.0.0/0"
 	dockerConfigDir          = "/etc/docker"
 	machineSecurityGroupName = "docker-machine"
-	dockerPort               = 2376
+)
+
+var (
+	dockerPort = 2376
+	swarmPort  = 3376
 )
 
 type Driver struct {
@@ -193,6 +197,21 @@ func (d *Driver) SetConfigFromFlags(flags drivers.DriverOptions) error {
 		return fmt.Errorf("amazonec2 driver requires either the --amazonec2-subnet-id or --amazonec2-vpc-id option")
 	}
 
+	if d.isSwarmMaster() {
+		u, err := url.Parse(d.SwarmHost)
+		if err != nil {
+			return fmt.Errorf("error parsing swarm host: %s", err)
+		}
+
+		parts := strings.Split(u.Host, ":")
+		port, err := strconv.Atoi(parts[1])
+		if err != nil {
+			return err
+		}
+
+		swarmPort = port
+	}
+
 	return nil
 }
 
@@ -626,32 +645,7 @@ func (d *Driver) configureSecurityGroup(groupName string) error {
 
 	d.SecurityGroupId = securityGroup.GroupId
 
-	perms := configureSecurityGroupPermissions(securityGroup)
-
-	// configure swarm permission if needed
-	if d.isSwarmMaster() {
-		u, err := url.Parse(d.SwarmHost)
-		if err != nil {
-			return fmt.Errorf("error authorizing port for swarm: %s", err)
-		}
-
-		parts := strings.Split(u.Host, ":")
-		port, err := strconv.Atoi(parts[1])
-		if err != nil {
-			return err
-		}
-
-		log.Debugf("authorizing swarm on port %d", port)
-
-		perms = append(perms, amz.IpPermission{
-			IpProtocol: "tcp",
-			FromPort:   port,
-			ToPort:     port,
-			IpRange:    ipRange,
-		})
-	}
-
-	log.Debugf("configuring security group authorization for %s", ipRange)
+	perms := d.configureSecurityGroupPermissions(securityGroup)
 
 	if len(perms) != 0 {
 		log.Debugf("authorizing group %s with permissions: %v", securityGroup.GroupName, perms)
@@ -664,42 +658,52 @@ func (d *Driver) configureSecurityGroup(groupName string) error {
 	return nil
 }
 
-func configureSecurityGroupPermissions(group *amz.SecurityGroup) []amz.IpPermission {
+func (d *Driver) configureSecurityGroupPermissions(group *amz.SecurityGroup) []amz.IpPermission {
 	hasSshPort := false
 	hasDockerPort := false
+	hasSwarmPort := false
 	for _, p := range group.IpPermissions {
 		switch p.FromPort {
 		case 22:
 			hasSshPort = true
 		case dockerPort:
 			hasDockerPort = true
+		case swarmPort:
+			hasSwarmPort = true
 		}
 	}
 
 	perms := []amz.IpPermission{}
 
 	if !hasSshPort {
-		perm := amz.IpPermission{
+		perms = append(perms, amz.IpPermission{
 			IpProtocol: "tcp",
 			FromPort:   22,
 			ToPort:     22,
 			IpRange:    ipRange,
-		}
-
-		perms = append(perms, perm)
+		})
 	}
 
 	if !hasDockerPort {
-		perm := amz.IpPermission{
+		perms = append(perms, amz.IpPermission{
 			IpProtocol: "tcp",
 			FromPort:   dockerPort,
 			ToPort:     dockerPort,
 			IpRange:    ipRange,
-		}
-
-		perms = append(perms, perm)
+		})
 	}
 
+	if !hasSwarmPort && d.SwarmMaster {
+		perms = append(perms, amz.IpPermission{
+			IpProtocol: "tcp",
+			FromPort:   swarmPort,
+			ToPort:     swarmPort,
+			IpRange:    ipRange,
+		})
+	}
+
+	log.Debugf("configuring security group authorization for %s", ipRange)
+
 	return perms
 }
 

drivers/amazonec2/amazonec2_test.go

@@ -1,11 +1,24 @@
 package amazonec2
 
 import (
+	"io/ioutil"
+	"os"
 	"testing"
 
 	"github.com/docker/machine/drivers/amazonec2/amz"
 )
 
+const (
+	testSshPort           = 22
+	testDockerPort        = 2376
+	testStoreDir          = ".store-test"
+	machineTestName       = "test-host"
+	machineTestDriverName = "none"
+	machineTestStorePath  = "/test/path"
+	machineTestCaCert     = "test-cert"
+	machineTestPrivateKey = "test-key"
+)
+
 var (
 	securityGroup = amz.SecurityGroup{
 		GroupName: "test-group",
@@ -14,20 +27,96 @@ var (
 	}
 )
 
-const (
-	testSshPort    = 22
-	testDockerPort = 2376
-)
+type DriverOptionsMock struct {
+	Data map[string]interface{}
+}
+
+func (d DriverOptionsMock) String(key string) string {
+	return d.Data[key].(string)
+}
+
+func (d DriverOptionsMock) Int(key string) int {
+	return d.Data[key].(int)
+}
+
+func (d DriverOptionsMock) Bool(key string) bool {
+	return d.Data[key].(bool)
+}
+
+func cleanup() error {
+	return os.RemoveAll(testStoreDir)
+}
+
+func getTestStorePath() (string, error) {
+	tmpDir, err := ioutil.TempDir("", "machine-test-")
+	if err != nil {
+		return "", err
+	}
+	os.Setenv("MACHINE_STORAGE_PATH", tmpDir)
+	return tmpDir, nil
+}
+
+func getDefaultTestDriverFlags() *DriverOptionsMock {
+	return &DriverOptionsMock{
+		Data: map[string]interface{}{
+			"name":                     "test",
+			"url":                      "unix:///var/run/docker.sock",
+			"swarm":                    false,
+			"swarm-host":               "",
+			"swarm-master":             false,
+			"swarm-discovery":          "",
+			"amazonec2-ami":            "ami-12345",
+			"amazonec2-access-key":     "abcdefg",
+			"amazonec2-secret-key":     "12345",
+			"amazonec2-session-token":  "",
+			"amazonec2-instance-type":  "t1.micro",
+			"amazonec2-vpc-id":         "vpc-12345",
+			"amazonec2-subnet-id":      "subnet-12345",
+			"amazonec2-security-group": "docker-machine-test",
+			"amazonec2-region":         "us-east-1",
+			"amazonec2-zone":           "e",
+			"amazonec2-root-size":      10,
+		},
+	}
+}
+
+func getTestDriver() (*Driver, error) {
+	storePath, err := getTestStorePath()
+	if err != nil {
+		return nil, err
+	}
+	defer cleanup()
+
+	d, err := NewDriver(machineTestName, storePath, machineTestCaCert, machineTestPrivateKey)
+	if err != nil {
+		return nil, err
+	}
+	d.SetConfigFromFlags(getDefaultTestDriverFlags())
+	drv := d.(*Driver)
+	return drv, nil
+}
 
 func TestConfigureSecurityGroupPermissionsEmpty(t *testing.T) {
+	d, err := getTestDriver()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer cleanup()
+
 	group := securityGroup
-	perms := configureSecurityGroupPermissions(&group)
+	perms := d.configureSecurityGroupPermissions(&group)
 	if len(perms) != 2 {
 		t.Fatalf("expected 2 permissions; received %d", len(perms))
 	}
 }
 
 func TestConfigureSecurityGroupPermissionsSshOnly(t *testing.T) {
+	d, err := getTestDriver()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer cleanup()
+
 	group := securityGroup
 
 	group.IpPermissions = []amz.IpPermission{
@@ -38,7 +127,7 @@ func TestConfigureSecurityGroupPermissionsSshOnly(t *testing.T) {
 		},
 	}
 
-	perms := configureSecurityGroupPermissions(&group)
+	perms := d.configureSecurityGroupPermissions(&group)
 	if len(perms) != 1 {
 		t.Fatalf("expected 1 permission; received %d", len(perms))
 	}
@@ -50,6 +139,12 @@ func TestConfigureSecurityGroupPermissionsSshOnly(t *testing.T) {
 }
 
 func TestConfigureSecurityGroupPermissionsDockerOnly(t *testing.T) {
+	d, err := getTestDriver()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer cleanup()
+
 	group := securityGroup
 
 	group.IpPermissions = []amz.IpPermission{
@@ -60,7 +155,7 @@ func TestConfigureSecurityGroupPermissionsDockerOnly(t *testing.T) {
 		},
 	}
 
-	perms := configureSecurityGroupPermissions(&group)
+	perms := d.configureSecurityGroupPermissions(&group)
 	if len(perms) != 1 {
 		t.Fatalf("expected 1 permission; received %d", len(perms))
 	}
@@ -72,6 +167,12 @@ func TestConfigureSecurityGroupPermissionsDockerOnly(t *testing.T) {
 }
 
 func TestConfigureSecurityGroupPermissionsDockerAndSsh(t *testing.T) {
+	d, err := getTestDriver()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer cleanup()
+
 	group := securityGroup
 
 	group.IpPermissions = []amz.IpPermission{
@@ -87,7 +188,7 @@ func TestConfigureSecurityGroupPermissionsDockerAndSsh(t *testing.T) {
 		},
 	}
 
-	perms := configureSecurityGroupPermissions(&group)
+	perms := d.configureSecurityGroupPermissions(&group)
 	if len(perms) != 0 {
 		t.Fatalf("expected 0 permissions; received %d", len(perms))
 	}