decouple buildLoadBalancerSubnets from buildLoadBalancerSubnetMappings

Author: oliviassss

pkg/service/model_build_load_balancer.go

@@ -30,32 +30,31 @@ const (
 	minimalAvailableIPAddressCount 		 = int64(8)
 )
 
-func (t *defaultModelBuildTask) buildLoadBalancer(ctx context.Context, scheme elbv2model.LoadBalancerScheme) ([]*ec2.Subnet, error) {
-	spec, ec2Subnets, err := t.buildLoadBalancerSpec(ctx, scheme)
+func (t *defaultModelBuildTask) buildLoadBalancer(ctx context.Context, scheme elbv2model.LoadBalancerScheme) error {
+	spec, err := t.buildLoadBalancerSpec(ctx, scheme)
 	if err != nil {
-		return nil, err
+		return err
 	}
 	t.loadBalancer = elbv2model.NewLoadBalancer(t.stack, resourceIDLoadBalancer, spec)
-	return ec2Subnets, nil
+	return nil
 }
 
-func (t *defaultModelBuildTask) buildLoadBalancerSpec(ctx context.Context, scheme elbv2model.LoadBalancerScheme) (elbv2model.LoadBalancerSpec, []*ec2.Subnet, error) {
+func (t *defaultModelBuildTask) buildLoadBalancerSpec(ctx context.Context, scheme elbv2model.LoadBalancerScheme) (elbv2model.LoadBalancerSpec, error) {
 	ipAddressType, err := t.buildLoadBalancerIPAddressType(ctx)
 	if err != nil {
-		return elbv2model.LoadBalancerSpec{}, nil, err
+		return elbv2model.LoadBalancerSpec{}, err
 	}
 	lbAttributes, err := t.buildLoadBalancerAttributes(ctx)
 	if err != nil {
-		return elbv2model.LoadBalancerSpec{}, nil, err
+		return elbv2model.LoadBalancerSpec{}, err
 	}
 	tags, err := t.buildLoadBalancerTags(ctx)
 	if err != nil {
-		return elbv2model.LoadBalancerSpec{}, nil, err
+		return elbv2model.LoadBalancerSpec{}, err
 	}
-
-	subnetMappings, ec2Subnets, err := t.buildLoadBalancerSubnetMappings(ctx, scheme)
+	subnetMappings, err := t.buildLoadBalancerSubnetMappings(ctx, scheme, t.ec2Subnets)
 	if err != nil {
-		return elbv2model.LoadBalancerSpec{}, nil, err
+		return elbv2model.LoadBalancerSpec{}, err
 	}
 	name := t.buildLoadBalancerName(ctx, scheme)
 	spec := elbv2model.LoadBalancerSpec{
@@ -67,7 +66,7 @@ func (t *defaultModelBuildTask) buildLoadBalancerSpec(ctx context.Context, schem
 		LoadBalancerAttributes: lbAttributes,
 		Tags:                   tags,
 	}
-	return spec, ec2Subnets, nil
+	return spec, nil
 }
 
 func (t *defaultModelBuildTask) buildLoadBalancerIPAddressType(_ context.Context) (elbv2model.IPAddressType, error) {
@@ -179,31 +178,29 @@ func (t *defaultModelBuildTask) buildLoadBalancerTags(ctx context.Context) (map[
 	return t.buildAdditionalResourceTags(ctx)
 }
 
-func (t *defaultModelBuildTask) buildLoadBalancerSubnetMappings(ctx context.Context, scheme elbv2model.LoadBalancerScheme) ([]elbv2model.SubnetMapping, []*ec2.Subnet, error) {
-
+func (t *defaultModelBuildTask) buildLoadBalancerSubnetMappings(ctx context.Context, scheme elbv2model.LoadBalancerScheme, ec2Subnets []*ec2.Subnet) ([]elbv2model.SubnetMapping, error) {
 	var eipAllocation []string
 	eipConfigured := t.annotationParser.ParseStringSliceAnnotation(annotations.SvcLBSuffixEIPAllocations, &eipAllocation, t.service.Annotations)
 	var privateIpv4Addresses []string
 	ipv4Configured := t.annotationParser.ParseStringSliceAnnotation(annotations.SvcLBSuffixPrivateIpv4Addresses, &privateIpv4Addresses, t.service.Annotations)
 
 	// Validation
 	if eipConfigured && ipv4Configured {
-		return []elbv2model.SubnetMapping{}, nil, errors.Errorf("only one of EIP allocations or PrivateIpv4Addresses can be set")
+		return []elbv2model.SubnetMapping{}, errors.Errorf("only one of EIP allocations or PrivateIpv4Addresses can be set")
 	}
-	if eipConfigured && scheme == elbv2model.LoadBalancerSchemeInternal {
-		return []elbv2model.SubnetMapping{}, nil, errors.Errorf("EIP allocations can only be set for internet facing load balancers")
-	}
-	if ipv4Configured && scheme == elbv2model.LoadBalancerSchemeInternetFacing {
-		return []elbv2model.SubnetMapping{}, nil, errors.Errorf("PrivateIpv4Addresses can only be set for internal balancers")
-	}
-
-	ec2Subnets, _ := t.buildLoadBalancerSubnets(ctx, scheme, ipv4Configured)
-	if eipConfigured && len(eipAllocation) != len(ec2Subnets) {
-		return []elbv2model.SubnetMapping{}, nil, errors.Errorf("number of EIP allocations (%d) and subnets (%d) must match", len(eipAllocation), len(ec2Subnets))
+	if eipConfigured {
+		if scheme == elbv2model.LoadBalancerSchemeInternal {
+			return []elbv2model.SubnetMapping{}, errors.Errorf("EIP allocations can only be set for internet facing load balancers")
+		} else if len(eipAllocation) != len(ec2Subnets) {
+			return []elbv2model.SubnetMapping{}, errors.Errorf("number of EIP allocations (%d) and subnets (%d) must match", len(eipAllocation), len(ec2Subnets))
+		}
 	}
-
-	if ipv4Configured && len(privateIpv4Addresses) != len(ec2Subnets) {
-		return []elbv2model.SubnetMapping{}, nil, errors.Errorf("number of PrivateIpv4Addresses (%d) and subnets (%d) must match", len(privateIpv4Addresses), len(ec2Subnets))
+	if ipv4Configured {
+		if scheme == elbv2model.LoadBalancerSchemeInternetFacing {
+			return []elbv2model.SubnetMapping{}, errors.Errorf("PrivateIpv4Addresses can only be set for internal balancers")
+		} else if len(privateIpv4Addresses) != len(ec2Subnets) {
+			return []elbv2model.SubnetMapping{}, errors.Errorf("number of PrivateIpv4Addresses (%d) and subnets (%d) must match", len(privateIpv4Addresses), len(ec2Subnets))
+		}
 	}
 
 	subnetMappings := make([]elbv2model.SubnetMapping, 0, len(ec2Subnets))
@@ -217,13 +214,13 @@ func (t *defaultModelBuildTask) buildLoadBalancerSubnetMappings(ctx context.Cont
 		if ipv4Configured {
 			ip, err := t.getMatchingIPforSubnet(ctx, subnet, privateIpv4Addresses)
 			if err != nil {
-				return []elbv2model.SubnetMapping{}, nil, err
+				return []elbv2model.SubnetMapping{}, err
 			}
 			mapping.PrivateIPv4Address = aws.String(ip)
 		}
 		subnetMappings = append(subnetMappings, mapping)
 	}
-	return subnetMappings, ec2Subnets, nil
+	return subnetMappings, nil
 }
 
 // Return the ip address which is in the subnet. Error if not match
@@ -245,7 +242,7 @@ func (t *defaultModelBuildTask) getMatchingIPforSubnet(_ context.Context, subnet
 	return "", errors.Errorf("no matching ip for subnet %s", *subnet.SubnetId)
 }
 
-func (t *defaultModelBuildTask) buildLoadBalancerSubnets(ctx context.Context, scheme elbv2model.LoadBalancerScheme, ipv4Configured bool) ([]*ec2.Subnet, error) {
+func (t *defaultModelBuildTask) buildLoadBalancerSubnets(ctx context.Context, scheme elbv2model.LoadBalancerScheme) ([]*ec2.Subnet, error) {
 	var rawSubnetNameOrIDs []string
 	if exists := t.annotationParser.ParseStringSliceAnnotation(annotations.SvcLBSuffixSubnets, &rawSubnetNameOrIDs, t.service.Annotations); exists {
 		return t.subnetsResolver.ResolveViaNameOrIDSlice(ctx, rawSubnetNameOrIDs,
@@ -273,6 +270,8 @@ func (t *defaultModelBuildTask) buildLoadBalancerSubnets(ctx context.Context, sc
 
 	// for internet-facing Load Balancers, the subnets mush have at least 8 available IP addresses;
 	// for internal Load Balancers, this is only required if private ip address is not assigned
+	var privateIpv4Addresses []string
+	ipv4Configured := t.annotationParser.ParseStringSliceAnnotation(annotations.SvcLBSuffixPrivateIpv4Addresses, &privateIpv4Addresses, t.service.Annotations)
 	if (scheme == elbv2model.LoadBalancerSchemeInternetFacing) ||
 		((scheme == elbv2model.LoadBalancerSchemeInternal) && !ipv4Configured) {
 		return t.subnetsResolver.ResolveViaDiscovery(ctx,