Merge pull request kubernetes#23718 from justinsb/automated-cherry-pi…

…ck-of-#23340-upstream-release-1.2 Automatic merge from submit-queue Automated cherry pick of kubernetes#23340 Cherry pick of kubernetes#23340 on release-1.2.
shyamjvs · Apr 1, 2016 · 0eca3c7 · 0eca3c7
2 parents 5439f75 + cea1d80
commit 0eca3c7
Showing 1 changed file with 23 additions and 18 deletions.
diff --git a/pkg/cloudprovider/providers/aws/aws.go b/pkg/cloudprovider/providers/aws/aws.go
@@ -2286,37 +2286,42 @@ func toStatus(lb *elb.LoadBalancerDescription) *api.LoadBalancerStatus {
 // Otherwise we will return an error.
 func findSecurityGroupForInstance(instance *ec2.Instance, taggedSecurityGroups map[string]*ec2.SecurityGroup) (*ec2.GroupIdentifier, error) {
 	instanceID := aws.StringValue(instance.InstanceId)
-	var best *ec2.GroupIdentifier
+
+	var tagged []*ec2.GroupIdentifier
+	var untagged []*ec2.GroupIdentifier
 	for _, group := range instance.SecurityGroups {
 		groupID := aws.StringValue(group.GroupId)
 		if groupID == "" {
 			glog.Warningf("Ignoring security group without id for instance %q: %v", instanceID, group)
 			continue
 		}
-		if best == nil {
-			best = group
-			continue
+		_, isTagged := taggedSecurityGroups[groupID]
+		if isTagged {
+			tagged = append(tagged, group)
+		} else {
+			untagged = append(untagged, group)
 		}
+	}
 
-		_, bestIsTagged := taggedSecurityGroups[*best.GroupId]
-		_, groupIsTagged := taggedSecurityGroups[groupID]
-
-		if bestIsTagged && !groupIsTagged {
-			// best is still best
-		} else if groupIsTagged && !bestIsTagged {
-			best = group
-		} else {
-			// We create instances with one SG
-			// If users create multiple SGs, they must tag one of them as being k8s owned
-			return nil, fmt.Errorf("Multiple security groups found for instance (%s); ensure the k8s security group is tagged", instanceID)
+	if len(tagged) > 0 {
+		// We create instances with one SG
+		// If users create multiple SGs, they must tag one of them as being k8s owned
+		if len(tagged) != 1 {
+			return nil, fmt.Errorf("Multiple tagged security groups found for instance %s; ensure only the k8s security group is tagged", instanceID)
 		}
+		return tagged[0], nil
 	}
 
-	if best == nil {
-		glog.Warning("No security group found for instance ", instanceID)
+	if len(untagged) > 0 {
+		// For back-compat, we will allow a single untagged SG
+		if len(untagged) != 1 {
+			return nil, fmt.Errorf("Multiple untagged security groups found for instance %s; ensure the k8s security group is tagged", instanceID)
+		}
+		return untagged[0], nil
 	}
 
-	return best, nil
+	glog.Warningf("No security group found for instance %q", instanceID)
+	return nil, nil
 }
 
 // Return all the security groups that are tagged as being part of our cluster