diff --git a/pkg/cloudprovider/providers/aws/aws.go b/pkg/cloudprovider/providers/aws/aws.go index 268bef4b4bf..fb85af28153 100644 --- a/pkg/cloudprovider/providers/aws/aws.go +++ b/pkg/cloudprovider/providers/aws/aws.go @@ -172,6 +172,7 @@ type InstanceGroupInfo interface { type AWSCloud struct { awsServices AWSServices ec2 EC2 + elb ELB asg ASG cfg *AWSCloudConfig availabilityZone string @@ -183,8 +184,6 @@ type AWSCloud struct { selfAWSInstance *awsInstance mutex sync.Mutex - // Protects elbClients - elbClients map[string]ELB } type AWSCloudConfig struct { @@ -236,23 +235,6 @@ func (p *awsSDKProvider) Metadata() AWSMetadata { return &awsSdkMetadata{} } -// Builds an ELB client for the specified region -func (s *AWSCloud) getELBClient(regionName string) (ELB, error) { - s.mutex.Lock() - defer s.mutex.Unlock() - - elbClient, found := s.elbClients[regionName] - if !found { - var err error - elbClient, err = s.awsServices.LoadBalancing(regionName) - if err != nil { - return nil, err - } - s.elbClients[regionName] = elbClient - } - return elbClient, nil -} - func stringPointerArray(orig []string) []*string { if orig == nil { return nil @@ -566,6 +548,11 @@ func newAWSCloud(config io.Reader, awsServices AWSServices) (*AWSCloud, error) { return nil, fmt.Errorf("error creating AWS EC2 client: %v", err) } + elb, err := awsServices.LoadBalancing(regionName) + if err != nil { + return nil, fmt.Errorf("error creating AWS ELB client: %v", err) + } + asg, err := awsServices.Autoscaling(regionName) if err != nil { return nil, fmt.Errorf("error creating AWS autoscaling client: %v", err) @@ -574,11 +561,11 @@ func newAWSCloud(config io.Reader, awsServices AWSServices) (*AWSCloud, error) { awsCloud := &AWSCloud{ awsServices: awsServices, ec2: ec2, + elb: elb, asg: asg, cfg: cfg, region: regionName, availabilityZone: zone, - elbClients: map[string]ELB{}, } filterTags := map[string]string{} @@ -1240,16 +1227,11 @@ func (v *AWSCloud) Release(name string) error { } // Gets the current load balancer state -func (s *AWSCloud) describeLoadBalancer(region, name string) (*elb.LoadBalancerDescription, error) { - elbClient, err := s.getELBClient(region) - if err != nil { - return nil, err - } - +func (s *AWSCloud) describeLoadBalancer(name string) (*elb.LoadBalancerDescription, error) { request := &elb.DescribeLoadBalancersInput{} request.LoadBalancerNames = []*string{&name} - response, err := elbClient.DescribeLoadBalancers(request) + response, err := s.elb.DescribeLoadBalancers(request) if err != nil { if awsError, ok := err.(awserr.Error); ok { if awsError.Code() == "LoadBalancerNotFound" { @@ -1269,19 +1251,6 @@ func (s *AWSCloud) describeLoadBalancer(region, name string) (*elb.LoadBalancerD return ret, nil } -// TCPLoadBalancerExists implements TCPLoadBalancer.TCPLoadBalancerExists. -func (self *AWSCloud) TCPLoadBalancerExists(name, region string) (bool, error) { - lb, err := self.describeLoadBalancer(name, region) - if err != nil { - return false, err - } - - if lb != nil { - return true, nil - } - return false, nil -} - // Retrieves instance's vpc id from metadata func (self *AWSCloud) findVPCID() (string, error) { @@ -1626,9 +1595,8 @@ func (s *AWSCloud) createTags(request *ec2.CreateTagsInput) (*ec2.CreateTagsOutp func (s *AWSCloud) EnsureTCPLoadBalancer(name, region string, publicIP net.IP, ports []*api.ServicePort, hosts []string, affinity api.ServiceAffinity) (*api.LoadBalancerStatus, error) { glog.V(2).Infof("EnsureTCPLoadBalancer(%v, %v, %v, %v, %v)", name, region, publicIP, ports, hosts) - elbClient, err := s.getELBClient(region) - if err != nil { - return nil, err + if region != s.region { + return nil, fmt.Errorf("requested load balancer region '%s' does not match cluster region '%s'", region, s.region) } if affinity != api.ServiceAffinityNone { @@ -1733,12 +1701,12 @@ func (s *AWSCloud) EnsureTCPLoadBalancer(name, region string, publicIP net.IP, p } // Build the load balancer itself - loadBalancer, err := s.ensureLoadBalancer(region, name, listeners, subnetIDs, securityGroupIDs) + loadBalancer, err := s.ensureLoadBalancer(name, listeners, subnetIDs, securityGroupIDs) if err != nil { return nil, err } - err = s.ensureLoadBalancerHealthCheck(region, loadBalancer, listeners) + err = s.ensureLoadBalancerHealthCheck(loadBalancer, listeners) if err != nil { return nil, err } @@ -1749,7 +1717,7 @@ func (s *AWSCloud) EnsureTCPLoadBalancer(name, region string, publicIP net.IP, p return nil, err } - err = s.ensureLoadBalancerInstances(elbClient, orEmpty(loadBalancer.LoadBalancerName), loadBalancer.Instances, instances) + err = s.ensureLoadBalancerInstances(orEmpty(loadBalancer.LoadBalancerName), loadBalancer.Instances, instances) if err != nil { glog.Warning("Error registering instances with the load balancer: %v", err) return nil, err @@ -1765,7 +1733,11 @@ func (s *AWSCloud) EnsureTCPLoadBalancer(name, region string, publicIP net.IP, p // GetTCPLoadBalancer is an implementation of TCPLoadBalancer.GetTCPLoadBalancer func (s *AWSCloud) GetTCPLoadBalancer(name, region string) (*api.LoadBalancerStatus, bool, error) { - lb, err := s.describeLoadBalancer(region, name) + if region != s.region { + return nil, false, fmt.Errorf("requested load balancer region '%s' does not match cluster region '%s'", region, s.region) + } + + lb, err := s.describeLoadBalancer(name) if err != nil { return nil, false, err } @@ -1925,12 +1897,11 @@ func (s *AWSCloud) updateInstanceSecurityGroupsForLoadBalancer(lb *elb.LoadBalan // EnsureTCPLoadBalancerDeleted implements TCPLoadBalancer.EnsureTCPLoadBalancerDeleted. func (s *AWSCloud) EnsureTCPLoadBalancerDeleted(name, region string) error { - elbClient, err := s.getELBClient(region) - if err != nil { - return err + if region != s.region { + return fmt.Errorf("requested load balancer region '%s' does not match cluster region '%s'", region, s.region) } - lb, err := s.describeLoadBalancer(region, name) + lb, err := s.describeLoadBalancer(name) if err != nil { return err } @@ -1954,7 +1925,7 @@ func (s *AWSCloud) EnsureTCPLoadBalancerDeleted(name, region string) error { request := &elb.DeleteLoadBalancerInput{} request.LoadBalancerName = lb.LoadBalancerName - _, err = elbClient.DeleteLoadBalancer(request) + _, err = s.elb.DeleteLoadBalancer(request) if err != nil { // TODO: Check if error was because load balancer was concurrently deleted glog.Error("error deleting load balancer: ", err) @@ -2020,17 +1991,16 @@ func (s *AWSCloud) EnsureTCPLoadBalancerDeleted(name, region string) error { // UpdateTCPLoadBalancer implements TCPLoadBalancer.UpdateTCPLoadBalancer func (s *AWSCloud) UpdateTCPLoadBalancer(name, region string, hosts []string) error { + if region != s.region { + return fmt.Errorf("requested load balancer region '%s' does not match cluster region '%s'", region, s.region) + } + instances, err := s.getInstancesByNodeNames(hosts) if err != nil { return err } - elbClient, err := s.getELBClient(region) - if err != nil { - return err - } - - lb, err := s.describeLoadBalancer(region, name) + lb, err := s.describeLoadBalancer(name) if err != nil { return err } @@ -2039,7 +2009,7 @@ func (s *AWSCloud) UpdateTCPLoadBalancer(name, region string, hosts []string) er return fmt.Errorf("Load balancer not found") } - err = s.ensureLoadBalancerInstances(elbClient, orEmpty(lb.LoadBalancerName), lb.Instances, instances) + err = s.ensureLoadBalancerInstances(orEmpty(lb.LoadBalancerName), lb.Instances, instances) if err != nil { return nil } diff --git a/pkg/cloudprovider/providers/aws/aws_loadbalancer.go b/pkg/cloudprovider/providers/aws/aws_loadbalancer.go index c3469bcddb2..c96e1155176 100644 --- a/pkg/cloudprovider/providers/aws/aws_loadbalancer.go +++ b/pkg/cloudprovider/providers/aws/aws_loadbalancer.go @@ -27,13 +27,8 @@ import ( "k8s.io/kubernetes/pkg/util/sets" ) -func (s *AWSCloud) ensureLoadBalancer(region, name string, listeners []*elb.Listener, subnetIDs []string, securityGroupIDs []string) (*elb.LoadBalancerDescription, error) { - elbClient, err := s.getELBClient(region) - if err != nil { - return nil, err - } - - loadBalancer, err := s.describeLoadBalancer(region, name) +func (s *AWSCloud) ensureLoadBalancer(name string, listeners []*elb.Listener, subnetIDs []string, securityGroupIDs []string) (*elb.LoadBalancerDescription, error) { + loadBalancer, err := s.describeLoadBalancer(name) if err != nil { return nil, err } @@ -53,7 +48,7 @@ func (s *AWSCloud) ensureLoadBalancer(region, name string, listeners []*elb.List createRequest.SecurityGroups = stringPointerArray(securityGroupIDs) glog.Info("Creating load balancer with name: ", name) - _, err := elbClient.CreateLoadBalancer(createRequest) + _, err := s.elb.CreateLoadBalancer(createRequest) if err != nil { return nil, err } @@ -72,7 +67,7 @@ func (s *AWSCloud) ensureLoadBalancer(region, name string, listeners []*elb.List request.LoadBalancerName = aws.String(name) request.Subnets = stringSetToPointers(removals) glog.V(2).Info("Detaching load balancer from removed subnets") - _, err := elbClient.DetachLoadBalancerFromSubnets(request) + _, err := s.elb.DetachLoadBalancerFromSubnets(request) if err != nil { return nil, fmt.Errorf("error detaching AWS loadbalancer from subnets: %v", err) } @@ -84,7 +79,7 @@ func (s *AWSCloud) ensureLoadBalancer(region, name string, listeners []*elb.List request.LoadBalancerName = aws.String(name) request.Subnets = stringSetToPointers(additions) glog.V(2).Info("Attaching load balancer to added subnets") - _, err := elbClient.AttachLoadBalancerToSubnets(request) + _, err := s.elb.AttachLoadBalancerToSubnets(request) if err != nil { return nil, fmt.Errorf("error attaching AWS loadbalancer to subnets: %v", err) } @@ -103,7 +98,7 @@ func (s *AWSCloud) ensureLoadBalancer(region, name string, listeners []*elb.List request.LoadBalancerName = aws.String(name) request.SecurityGroups = stringPointerArray(securityGroupIDs) glog.V(2).Info("Applying updated security groups to load balancer") - _, err := elbClient.ApplySecurityGroupsToLoadBalancer(request) + _, err := s.elb.ApplySecurityGroupsToLoadBalancer(request) if err != nil { return nil, fmt.Errorf("error applying AWS loadbalancer security groups: %v", err) } @@ -163,7 +158,7 @@ func (s *AWSCloud) ensureLoadBalancer(region, name string, listeners []*elb.List request.LoadBalancerName = aws.String(name) request.LoadBalancerPorts = removals glog.V(2).Info("Deleting removed load balancer listeners") - _, err := elbClient.DeleteLoadBalancerListeners(request) + _, err := s.elb.DeleteLoadBalancerListeners(request) if err != nil { return nil, fmt.Errorf("error deleting AWS loadbalancer listeners: %v", err) } @@ -175,7 +170,7 @@ func (s *AWSCloud) ensureLoadBalancer(region, name string, listeners []*elb.List request.LoadBalancerName = aws.String(name) request.Listeners = additions glog.V(2).Info("Creating added load balancer listeners") - _, err := elbClient.CreateLoadBalancerListeners(request) + _, err := s.elb.CreateLoadBalancerListeners(request) if err != nil { return nil, fmt.Errorf("error creating AWS loadbalancer listeners: %v", err) } @@ -185,7 +180,7 @@ func (s *AWSCloud) ensureLoadBalancer(region, name string, listeners []*elb.List } if dirty { - loadBalancer, err = s.describeLoadBalancer(region, name) + loadBalancer, err = s.describeLoadBalancer(name) if err != nil { glog.Warning("Unable to retrieve load balancer after creation/update") return nil, err @@ -196,12 +191,7 @@ func (s *AWSCloud) ensureLoadBalancer(region, name string, listeners []*elb.List } // Makes sure that the health check for an ELB matches the configured listeners -func (s *AWSCloud) ensureLoadBalancerHealthCheck(region string, loadBalancer *elb.LoadBalancerDescription, listeners []*elb.Listener) error { - elbClient, err := s.getELBClient(region) - if err != nil { - return err - } - +func (s *AWSCloud) ensureLoadBalancerHealthCheck(loadBalancer *elb.LoadBalancerDescription, listeners []*elb.Listener) error { actual := loadBalancer.HealthCheck // Default AWS settings @@ -245,7 +235,7 @@ func (s *AWSCloud) ensureLoadBalancerHealthCheck(region string, loadBalancer *el request.HealthCheck = healthCheck request.LoadBalancerName = loadBalancer.LoadBalancerName - _, err = elbClient.ConfigureHealthCheck(request) + _, err := s.elb.ConfigureHealthCheck(request) if err != nil { return fmt.Errorf("error configuring load-balancer health-check: %v", err) } @@ -254,7 +244,7 @@ func (s *AWSCloud) ensureLoadBalancerHealthCheck(region string, loadBalancer *el } // Makes sure that exactly the specified hosts are registered as instances with the load balancer -func (s *AWSCloud) ensureLoadBalancerInstances(elbClient ELB, loadBalancerName string, lbInstances []*elb.Instance, instances []*ec2.Instance) error { +func (s *AWSCloud) ensureLoadBalancerInstances(loadBalancerName string, lbInstances []*elb.Instance, instances []*ec2.Instance) error { expected := sets.NewString() for _, instance := range instances { expected.Insert(orEmpty(instance.InstanceID)) @@ -286,7 +276,7 @@ func (s *AWSCloud) ensureLoadBalancerInstances(elbClient ELB, loadBalancerName s registerRequest := &elb.RegisterInstancesWithLoadBalancerInput{} registerRequest.Instances = addInstances registerRequest.LoadBalancerName = aws.String(loadBalancerName) - _, err := elbClient.RegisterInstancesWithLoadBalancer(registerRequest) + _, err := s.elb.RegisterInstancesWithLoadBalancer(registerRequest) if err != nil { return err } @@ -297,7 +287,7 @@ func (s *AWSCloud) ensureLoadBalancerInstances(elbClient ELB, loadBalancerName s deregisterRequest := &elb.DeregisterInstancesFromLoadBalancerInput{} deregisterRequest.Instances = removeInstances deregisterRequest.LoadBalancerName = aws.String(loadBalancerName) - _, err := elbClient.DeregisterInstancesFromLoadBalancer(deregisterRequest) + _, err := s.elb.DeregisterInstancesFromLoadBalancer(deregisterRequest) if err != nil { return err } diff --git a/pkg/cloudprovider/providers/aws/aws_test.go b/pkg/cloudprovider/providers/aws/aws_test.go index 07171a11827..67fa86de33f 100644 --- a/pkg/cloudprovider/providers/aws/aws_test.go +++ b/pkg/cloudprovider/providers/aws/aws_test.go @@ -17,6 +17,7 @@ limitations under the License. package aws_cloud import ( + "fmt" "io" "reflect" "strings" @@ -664,3 +665,35 @@ func TestFindVPCID(t *testing.T) { t.Errorf("Unexpected vpcID: %s", vpcID) } } + +func TestLoadBalancerMatchesClusterRegion(t *testing.T) { + awsServices := NewFakeAWSServices() + c, err := newAWSCloud(strings.NewReader("[global]"), awsServices) + if err != nil { + t.Errorf("Error building aws cloud: %v", err) + return + } + + badELBRegion := "bad-elb-region" + errorMessage := fmt.Sprintf("requested load balancer region '%s' does not match cluster region '%s'", badELBRegion, c.region) + + _, _, err = c.GetTCPLoadBalancer("elb-name", badELBRegion) + if err == nil || err.Error() != errorMessage { + t.Errorf("Expected GetTCPLoadBalancer region mismatch error.") + } + + _, err = c.EnsureTCPLoadBalancer("elb-name", badELBRegion, nil, nil, nil, api.ServiceAffinityNone) + if err == nil || err.Error() != errorMessage { + t.Errorf("Expected EnsureTCPLoadBalancer region mismatch error.") + } + + err = c.EnsureTCPLoadBalancerDeleted("elb-name", badELBRegion) + if err == nil || err.Error() != errorMessage { + t.Errorf("Expected EnsureTCPLoadBalancerDeleted region mismatch error.") + } + + err = c.UpdateTCPLoadBalancer("elb-name", badELBRegion, nil) + if err == nil || err.Error() != errorMessage { + t.Errorf("Expected UpdateTCPLoadBalancer region mismatch error.") + } +}