From ddaac519dcea65f0202d623072c4708fe5440383 Mon Sep 17 00:00:00 2001 From: Nathan Button Date: Sat, 29 Apr 2017 10:12:48 -0600 Subject: [PATCH 1/2] If ElbSecurityGroup is set then use it instead of creating another SG --- pkg/cloudprovider/providers/aws/aws.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/pkg/cloudprovider/providers/aws/aws.go b/pkg/cloudprovider/providers/aws/aws.go index 85af7bacba1..a728b431ee2 100644 --- a/pkg/cloudprovider/providers/aws/aws.go +++ b/pkg/cloudprovider/providers/aws/aws.go @@ -412,6 +412,11 @@ type CloudConfig struct { //local VPC subnet (so load balancers can access it). E.g. 10.82.0.0/16 30000-32000. DisableSecurityGroupIngress bool + //AWS has a hard limit of 500 security groups. For large clusters creating a security group for each ELB + //can cause the max number of security groups to be reached. If this is set instead of creating a new + //Security group for each ELB this security group will be used instead. + ElbSecurityGroup string + //During the instantiation of an new AWS cloud provider, the detected region //is validated against a known set of regions. // @@ -2724,7 +2729,10 @@ func (c *Cloud) EnsureLoadBalancer(clusterName string, apiService *v1.Service, n // Create a security group for the load balancer var securityGroupID string - { + if c.cfg.Global.ElbSecurityGroup != "" { + securityGroupID = c.cfg.Global.ElbSecurityGroup + } else { + sgName := "k8s-elb-" + loadBalancerName sgDescription := fmt.Sprintf("Security group for Kubernetes ELB %s (%v)", loadBalancerName, serviceName) securityGroupID, err = c.ensureSecurityGroup(sgName, sgDescription) @@ -3084,6 +3092,10 @@ func (c *Cloud) EnsureLoadBalancerDeleted(clusterName string, service *v1.Servic // Collect the security groups to delete securityGroupIDs := map[string]struct{}{} for _, securityGroupID := range lb.SecurityGroups { + if *securityGroupID == c.cfg.Global.ElbSecurityGroup { + //We don't want to delete a security group that was defined in the Cloud Configurationn. + continue + } if isNilOrEmpty(securityGroupID) { glog.Warning("Ignoring empty security group in ", service.Name) continue From 06779586cdd712d8e824417d5b65b344f86816b3 Mon Sep 17 00:00:00 2001 From: Nathan Button Date: Wed, 3 May 2017 13:51:58 -0600 Subject: [PATCH 2/2] Clean up and restructure. --- pkg/cloudprovider/providers/aws/aws.go | 85 +++++++++++++------------- 1 file changed, 44 insertions(+), 41 deletions(-) diff --git a/pkg/cloudprovider/providers/aws/aws.go b/pkg/cloudprovider/providers/aws/aws.go index a728b431ee2..8858b413ed0 100644 --- a/pkg/cloudprovider/providers/aws/aws.go +++ b/pkg/cloudprovider/providers/aws/aws.go @@ -2729,52 +2729,55 @@ func (c *Cloud) EnsureLoadBalancer(clusterName string, apiService *v1.Service, n // Create a security group for the load balancer var securityGroupID string - if c.cfg.Global.ElbSecurityGroup != "" { - securityGroupID = c.cfg.Global.ElbSecurityGroup - } else { + { + if c.cfg.Global.ElbSecurityGroup != "" { + securityGroupID = c.cfg.Global.ElbSecurityGroup - sgName := "k8s-elb-" + loadBalancerName - sgDescription := fmt.Sprintf("Security group for Kubernetes ELB %s (%v)", loadBalancerName, serviceName) - securityGroupID, err = c.ensureSecurityGroup(sgName, sgDescription) - if err != nil { - glog.Error("Error creating load balancer security group: ", err) - return nil, err - } + } else { - ec2SourceRanges := []*ec2.IpRange{} - for _, sourceRange := range sourceRanges.StringSlice() { - ec2SourceRanges = append(ec2SourceRanges, &ec2.IpRange{CidrIp: aws.String(sourceRange)}) - } - - permissions := NewIPPermissionSet() - for _, port := range apiService.Spec.Ports { - portInt64 := int64(port.Port) - protocol := strings.ToLower(string(port.Protocol)) - - permission := &ec2.IpPermission{} - permission.FromPort = &portInt64 - permission.ToPort = &portInt64 - permission.IpRanges = ec2SourceRanges - permission.IpProtocol = &protocol - - permissions.Insert(permission) - } - - // Allow ICMP fragmentation packets, important for MTU discovery - { - permission := &ec2.IpPermission{ - IpProtocol: aws.String("icmp"), - FromPort: aws.Int64(3), - ToPort: aws.Int64(4), - IpRanges: []*ec2.IpRange{{CidrIp: aws.String("0.0.0.0/0")}}, + sgName := "k8s-elb-" + loadBalancerName + sgDescription := fmt.Sprintf("Security group for Kubernetes ELB %s (%v)", loadBalancerName, serviceName) + securityGroupID, err = c.ensureSecurityGroup(sgName, sgDescription) + if err != nil { + glog.Error("Error creating load balancer security group: ", err) + return nil, err } - permissions.Insert(permission) - } + ec2SourceRanges := []*ec2.IpRange{} + for _, sourceRange := range sourceRanges.StringSlice() { + ec2SourceRanges = append(ec2SourceRanges, &ec2.IpRange{CidrIp: aws.String(sourceRange)}) + } - _, err = c.setSecurityGroupIngress(securityGroupID, permissions) - if err != nil { - return nil, err + permissions := NewIPPermissionSet() + for _, port := range apiService.Spec.Ports { + portInt64 := int64(port.Port) + protocol := strings.ToLower(string(port.Protocol)) + + permission := &ec2.IpPermission{} + permission.FromPort = &portInt64 + permission.ToPort = &portInt64 + permission.IpRanges = ec2SourceRanges + permission.IpProtocol = &protocol + + permissions.Insert(permission) + } + + // Allow ICMP fragmentation packets, important for MTU discovery + { + permission := &ec2.IpPermission{ + IpProtocol: aws.String("icmp"), + FromPort: aws.Int64(3), + ToPort: aws.Int64(4), + IpRanges: []*ec2.IpRange{{CidrIp: aws.String("0.0.0.0/0")}}, + } + + permissions.Insert(permission) + } + + _, err = c.setSecurityGroupIngress(securityGroupID, permissions) + if err != nil { + return nil, err + } } } securityGroupIDs := []string{securityGroupID}