From eb6c77f08972a3282b7eb0449e37b28900dcd70b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kai=20L=C3=BCke?= <kai@kinvolk.io>
Date: Mon, 3 Feb 2020 21:05:51 +0100
Subject: [PATCH] platform/api/aws: Iterate over all subnets to run the
 instances

The subnet determines the Availability Zone. If in that
zone no machines are available, running an instance
will fail. The other Availability Zones probably have
machines available and the code should try to run
instances there instead of giving up too early.
---
 platform/api/aws/ec2.go     | 87 ++++++++++++++++++++-----------------
 platform/api/aws/network.go | 17 +++++---
 2 files changed, 60 insertions(+), 44 deletions(-)

diff --git a/platform/api/aws/ec2.go b/platform/api/aws/ec2.go
index 126de763e..fb0adc6cc 100644
--- a/platform/api/aws/ec2.go
+++ b/platform/api/aws/ec2.go
@@ -106,57 +106,66 @@ func (a *API) CreateInstances(name, keyname, userdata string, count uint64) ([]*
 		return nil, fmt.Errorf("error resolving vpc: %v", err)
 	}
 
-	subnetId, err := a.getSubnetID(vpcId)
+	subnetIds, err := a.getSubnetIDs(vpcId)
 	if err != nil {
-		return nil, fmt.Errorf("error resolving subnet: %v", err)
+		return nil, fmt.Errorf("error resolving subnets: %v", err)
 	}
 
 	key := &keyname
 	if keyname == "" {
 		key = nil
 	}
-	inst := ec2.RunInstancesInput{
-		ImageId:          &a.opts.AMI,
-		MinCount:         &cnt,
-		MaxCount:         &cnt,
-		KeyName:          key,
-		InstanceType:     &a.opts.InstanceType,
-		SecurityGroupIds: []*string{&sgId},
-		SubnetId:         &subnetId,
-		UserData:         ud,
-		IamInstanceProfile: &ec2.IamInstanceProfileSpecification{
-			Name: &a.opts.IAMInstanceProfile,
-		},
-		TagSpecifications: []*ec2.TagSpecification{
-			&ec2.TagSpecification{
-				ResourceType: aws.String(ec2.ResourceTypeInstance),
-				Tags: []*ec2.Tag{
-					&ec2.Tag{
-						Key:   aws.String("Name"),
-						Value: aws.String(name),
-					},
-					&ec2.Tag{
-						Key:   aws.String("CreatedBy"),
-						Value: aws.String("mantle"),
+
+	var reservations *ec2.Reservation
+
+	for _, subnetId := range subnetIds {
+		inst := ec2.RunInstancesInput{
+			ImageId:          &a.opts.AMI,
+			MinCount:         &cnt,
+			MaxCount:         &cnt,
+			KeyName:          key,
+			InstanceType:     &a.opts.InstanceType,
+			SecurityGroupIds: []*string{&sgId},
+			SubnetId:         &subnetId,
+			UserData:         ud,
+			IamInstanceProfile: &ec2.IamInstanceProfileSpecification{
+				Name: &a.opts.IAMInstanceProfile,
+			},
+			TagSpecifications: []*ec2.TagSpecification{
+				&ec2.TagSpecification{
+					ResourceType: aws.String(ec2.ResourceTypeInstance),
+					Tags: []*ec2.Tag{
+						&ec2.Tag{
+							Key:   aws.String("Name"),
+							Value: aws.String(name),
+						},
+						&ec2.Tag{
+							Key:   aws.String("CreatedBy"),
+							Value: aws.String("mantle"),
+						},
 					},
 				},
 			},
-		},
-	}
+		}
 
-	var reservations *ec2.Reservation
-	err = util.RetryConditional(5, 5*time.Second, func(err error) bool {
-		// due to AWS' eventual consistency despite ensuring that the IAM Instance
-		// Profile has been created it may not be available to ec2 yet.
-		if awsErr, ok := err.(awserr.Error); ok && (awsErr.Code() == "InvalidParameterValue" && strings.Contains(awsErr.Message(), "iamInstanceProfile.name")) {
-			return true
+		err = util.RetryConditional(5, 5*time.Second, func(err error) bool {
+			// due to AWS' eventual consistency despite ensuring that the IAM Instance
+			// Profile has been created it may not be available to ec2 yet.
+			if awsErr, ok := err.(awserr.Error); ok && (awsErr.Code() == "InvalidParameterValue" && strings.Contains(awsErr.Message(), "iamInstanceProfile.name")) {
+				return true
+			}
+			return false
+		}, func() error {
+			var ierr error
+			reservations, ierr = a.ec2.RunInstances(&inst)
+			return ierr
+		})
+
+		if err == nil {
+			break
 		}
-		return false
-	}, func() error {
-		var ierr error
-		reservations, ierr = a.ec2.RunInstances(&inst)
-		return ierr
-	})
+	}
+
 	if err != nil {
 		return nil, fmt.Errorf("error running instances: %v", err)
 	}
diff --git a/platform/api/aws/network.go b/platform/api/aws/network.go
index 9ce1a66a4..df2746f6a 100644
--- a/platform/api/aws/network.go
+++ b/platform/api/aws/network.go
@@ -304,8 +304,8 @@ func (a *API) createSubnets(vpcId, routeTableId string) error {
 	return nil
 }
 
-// getSubnetID gets a subnet for the given VPC.
-func (a *API) getSubnetID(vpc string) (string, error) {
+// getSubnetIDs gets the subnets for the given VPC.
+func (a *API) getSubnetIDs(vpc string) ([]string, error) {
 	subIds, err := a.ec2.DescribeSubnets(&ec2.DescribeSubnetsInput{
 		Filters: []*ec2.Filter{
 			{
@@ -315,14 +315,21 @@ func (a *API) getSubnetID(vpc string) (string, error) {
 		},
 	})
 	if err != nil {
-		return "", fmt.Errorf("unable to get subnets for vpc %v: %v", vpc, err)
+		return nil, fmt.Errorf("unable to get subnets for vpc %v: %v", vpc, err)
 	}
+
+	var ids []string
+
 	for _, id := range subIds.Subnets {
 		if id.SubnetId != nil {
-			return *id.SubnetId, nil
+			ids = append(ids, *id.SubnetId)
 		}
 	}
-	return "", fmt.Errorf("no subnets found for vpc %v", vpc)
+	if len(ids) == 0 {
+		return nil, fmt.Errorf("no subnets found for vpc %v", vpc)
+	}
+
+	return ids, nil
 }
 
 // getVPCID gets a VPC for the given security group