Changing externalSubnets leads to update failure

[lukehoban]

Changing externalSubnets causes a failure like below.

    +-aws:elasticloadbalancingv2/loadBalancer:LoadBalancer: (replace)
        [id=arn:aws:elasticloadbalancing:us-east-1:557981933943:loadbalancer/app/pulumi-ct-ae1-e97f95a/f02634ac0999337e]
        [urn=urn:pulumi:cts-prod::cts::aws:elasticloadbalancingv2/loadBalancer:LoadBalancer::pulumi-ct-ae1]
        enableDeletionProtection: false
        idleTimeout             : 60
        internal                : false
        loadBalancerType        : "application"
      - name                    : "pulumi-ct-ae1-e97f95a"
      + name                    : "pulumi-ct-ae1-c67d8ae"
        securityGroups          : [
            [0]: "sg-ec1d2698"
        ]
      ~ subnetMapping           : [
          ~ [0]: {
                  - subnetId: "subnet-496d3400"
                  + subnetId: "subnet-c293f299"
                }
            [1]: {
                    subnetId: "subnet-496d3400"
                }
            [2]: {
                    subnetId: "subnet-be69db82"
                }
        ]
        ---outputs:---
        arn                     : "arn:aws:elasticloadbalancing:us-east-1:557981933943:loadbalancer/app/pulumi-ct-ae1-c67d8ae/b30dfe4bdc4ce11d"
        arnSuffix               : "app/pulumi-ct-ae1-c67d8ae/b30dfe4bdc4ce11d"
        dnsName                 : "pulumi-ct-ae1-c67d8ae-907310313.us-east-1.elb.amazonaws.com"
        id                      : "arn:aws:elasticloadbalancing:us-east-1:557981933943:loadbalancer/app/pulumi-ct-ae1-c67d8ae/b30dfe4bdc4ce11d"
        idleTimeout             : "60"
        ipAddressType           : "ipv4"
        subnetMapping           : [
            [0]: {
                subnetId    : "subnet-496d3400"
            }
            [1]: {
                subnetId    : "subnet-be69db82"
            }
            [2]: {
                subnetId    : "subnet-c293f299"
            }
        ]
        subnets                 : [
            [0]: "subnet-496d3400"
            [1]: "subnet-c293f299"
            [2]: "subnet-be69db82"
        ]
        vpcId                   : "vpc-41289d27"
        zoneId                  : "Z35SXDOTRQ7X7K"
error PU2003: Plan apply failed: rpc error: code = Unknown desc = creating urn:pulumi:cts-prod::cts::cloud:service:Service$aws:elasticloadbalancingv2/listener:Listener::pulumi-ct-ae0: Error creating LB Listener: TargetGroupAssociationLimit: Target group 'arn:aws:elasticloadbalancing:us-east-1:557981933943:targetgroup/pulumi-ct-ae0-5b15186/0b971a071e5d8fff' cannot be associated with more than one load balancer
    status code: 400, request id: 47db327c-014c-11e8-a3ae-9fcc2ba7e8b4
Step #27 failed [create-replacement]: this failure was catastrophic and the provider cannot guarantee recovery
info: 1 change performed:
    +-1 resource replaced
      25 resources unchanged
Update duration: 2m49.753806706s
A catastrophic error occurred; resources states may be unknown
rpc error: code = Unknown desc = creating urn:pulumi:cts-prod::cts::cloud:service:Service$aws:elasticloadbalancingv2/listener:Listener::pulumi-ct-ae0: Error creating LB Listener: TargetGroupAssociationLimit: Target group 'arn:aws:elasticloadbalancing:us-east-1:557981933943:targetgroup/pulumi-ct-ae0-5b15186/0b971a071e5d8fff' cannot be associated with more than one load balancer
    status code: 400, request id: 47db327c-014c-11e8-a3ae-9fcc2ba7e8b4

[lukehoban]

Here's what happens:

1. We have one service S1 running, with LoadBalancer LB1, TargetGroup TG1 targeting S1 , and Listener L1 connecting LB1 to TG1.
2. The new set of subnets forces LB1 to be replaced (subnets are fixed at LB creation time).
3. So we create a new LoadBalancer LB2.
4. The replaced LoadBalancer forces the L1 to be replaced.
5. So we create a listener L2, connecting LB2 to TG1.
6. We are now attempting to have L1(LB1->TG1) and L2(LB2->TG1). This is not allowed in AWS - any given TG can only have a single LB targeting it.

[lukehoban]

The root of the problem here is that creating a new LoadBalancer should force the creation of new TargetGroups for any targets associated with the LoadBalancer, but it does not because the dependency between these is "indirect" in that it is created implicitly be the creation of a Listener connecting the two.

[lukehoban]

/cc @mmdriley

[lukehoban]

@mmdriley noted that the "solution" to this in CloudFormation is mentioned here: https://forums.aws.amazon.com/thread.jspa?messageID=783997&tstart=0

In paritcular, in order to force the TargetGroup to be replaced whenever the LoadBalancer is replaced - it is necessary to have some input property that forces replacement depend on the LoadBalancer. In the case below, they choose to use the Name property and include the name of the LoadBalancer in it.

AWSTemplateFormatVersion: '2010-09-09'
Description: Test of an issue with CloudFormation TargetGroups
Parameters:
  ELBScheme:
    Type: String
    Default: internet-facing
    AllowedValues: [ 'internet-facing', 'internal']
  Subnet1:
    Type: AWS::EC2::Subnet::Id
  Subnet2:
    Type: AWS::EC2::Subnet::Id
  VPC:
    Type: AWS::EC2::VPC::Id
Resources:
  ELBv2:
    Type: AWS::ElasticLoadBalancingV2::LoadBalancer
    Properties:
      Subnets:
      - !Ref Subnet1
      - !Ref Subnet2
      Scheme: !Ref ELBScheme
  ELBTargetGroup:
    Type: AWS::ElasticLoadBalancingV2::TargetGroup
    Properties:
      Name: !Join
      - '-'
      - - 'TG1'
        - !Select [ 2, !Split [ '-', !GetAtt ELBv2.LoadBalancerName]]
      Port: '80'
      Protocol: HTTP
      VpcId: !Ref VPC
  ELBListener80:
    Type: AWS::ElasticLoadBalancingV2::Listener
    Properties:
      DefaultActions:
      - TargetGroupArn: !Ref 'ELBTargetGroup'
        Type: forward
      LoadBalancerArn: !Ref 'ELBv2'
      Port: '80'
      Protocol: HTTP

This is more or less just a hack to convince the planning engine to believe there is a Forces Replacement dependency between these two resources.

Notably, Terraform also doesn't have a first class way to express this. They've discussed it in hashicorp/terraform#8099.

In the short term, we may need to do something like the hack above.

In the longer term, we may need to add a concept of dependsOnWithReplacement that is like our current dependsOn, but additionally communicates that the source resource should observe a forced replacement if the target resource is replaced.

[joeduffy]

Related to this, but orthogonal, is the fact that our current load balancers are auto-named with global names. Which means (I think) that if you can end up accidentally "rotating" load balancers between listeners, if you rearrange your cloud services. I believe based on our experience yesterday that we should think of the load balancer and listener as being "owned" by the service that forces its creation. Did we file a separate issue to track this?

[lukehoban]

Opened #349 on that issue. (Note that in practice that would have not had any bearing on the issue we hit yesterday - certainly there are other cases where it would cause problems though).

[lukehoban]

Moving to 0.11, since this is not a commonly hit scenario - and we should think about whether doing pulumi/pulumi#838 will be a better solution here before making what would otherwise be a breaking change to resource naming.

[lukehoban]

I don't believe this issue is tracking any specific proposal at this point - pulumi/pulumi#838 and #349 are the two follow-ups here.