Merge pull request #16356 from justinsb/revisions_and_pruning

Generate revisions of NLB objects, and introduce cleanup phase

cloudmock/aws/mockautoscaling/attach.go

@@ -17,6 +17,7 @@ limitations under the License.
 package mockautoscaling
 
 import (
+	"context"
 	"fmt"
 
 	"github.com/aws/aws-sdk-go/aws"
@@ -52,7 +53,7 @@ func (m *MockAutoscaling) AttachLoadBalancersRequest(*autoscaling.AttachLoadBala
 	return nil, nil
 }
 
-func (m *MockAutoscaling) AttachLoadBalancerTargetGroups(request *autoscaling.AttachLoadBalancerTargetGroupsInput) (*autoscaling.AttachLoadBalancerTargetGroupsOutput, error) {
+func (m *MockAutoscaling) AttachLoadBalancerTargetGroupsWithContext(ctx aws.Context, request *autoscaling.AttachLoadBalancerTargetGroupsInput, opts ...request.Option) (*autoscaling.AttachLoadBalancerTargetGroupsOutput, error) {
 	m.mutex.Lock()
 	defer m.mutex.Unlock()
 
@@ -62,9 +63,13 @@ func (m *MockAutoscaling) AttachLoadBalancerTargetGroups(request *autoscaling.At
 
 	asg := m.Groups[name]
 	if asg == nil {
-		return nil, fmt.Errorf("Group %q not found", name)
+		return nil, fmt.Errorf("group %q not found", name)
 	}
 
-	asg.TargetGroupARNs = request.TargetGroupARNs
+	asg.TargetGroupARNs = append(asg.TargetGroupARNs, request.TargetGroupARNs...)
 	return &autoscaling.AttachLoadBalancerTargetGroupsOutput{}, nil
 }
+
+func (m *MockAutoscaling) AttachLoadBalancerTargetGroups(request *autoscaling.AttachLoadBalancerTargetGroupsInput) (*autoscaling.AttachLoadBalancerTargetGroupsOutput, error) {
+	return m.AttachLoadBalancerTargetGroupsWithContext(context.TODO(), request)
+}

cmd/kops/update_cluster.go

@@ -80,6 +80,12 @@ type UpdateClusterOptions struct {
 	// LifecycleOverrides is a slice of taskName=lifecycle name values.  This slice is used
 	// to populate the LifecycleOverrides struct member in ApplyClusterCmd struct.
 	LifecycleOverrides []string
+
+	// Prune is true if we should clean up any old revisions of objects.
+	// Typically this is done in after we have rolling-updated the cluster.
+	// The goal is that the cluster can keep running even during more disruptive
+	// infrastructure changes.
+	Prune bool
 }
 
 func (o *UpdateClusterOptions) InitDefaults() {
@@ -91,6 +97,8 @@ func (o *UpdateClusterOptions) InitDefaults() {
 	// By default we export a kubecfg, but it doesn't have a static/eternal credential in it any more.
 	o.CreateKubecfg = true
 
+	o.Prune = false
+
 	o.RunTasksOptions.InitDefaults()
 }
 
@@ -133,6 +141,8 @@ func NewCmdUpdateCluster(f *util.Factory, out io.Writer) *cobra.Command {
 	viper.BindEnv("lifecycle-overrides", "KOPS_LIFECYCLE_OVERRIDES")
 	cmd.RegisterFlagCompletionFunc("lifecycle-overrides", completeLifecycleOverrides)
 
+	cmd.Flags().BoolVar(&options.Prune, "prune", options.Prune, "Delete old revisions of cloud resources that were needed during an upgrade")
+
 	return cmd
 }
 
@@ -251,6 +261,11 @@ func RunUpdateCluster(ctx context.Context, f *util.Factory, out io.Writer, c *Up
 		}
 	}
 
+	deletionProcessing := fi.DeletionProcessingModeDeleteIfNotDeferrred
+	if c.Prune {
+		deletionProcessing = fi.DeletionProcessingModeDeleteIncludingDeferred
+	}
+
 	lifecycleOverrideMap := make(map[string]fi.Lifecycle)
 
 	for _, override := range c.LifecycleOverrides {
@@ -287,6 +302,7 @@ func RunUpdateCluster(ctx context.Context, f *util.Factory, out io.Writer, c *Up
 		TargetName:         targetName,
 		LifecycleOverrides: lifecycleOverrideMap,
 		GetAssets:          c.GetAssets,
+		DeletionProcessing: deletionProcessing,
 	}
 
 	if err := applyCmd.Run(ctx); err != nil {

docs/cli/kops_update_cluster.md

@@ -33,6 +33,7 @@ kops update cluster [CLUSTER] [flags]
       --lifecycle-overrides strings   comma separated list of phase overrides, example: SecurityGroups=Ignore,InternetGateway=ExistsAndWarnIfChanges
       --out string                    Path to write any local output
       --phase string                  Subset of tasks to run: cluster, network, security
+      --prune                         Delete old revisions of cloud resources that were needed during an upgrade
       --ssh-public-key string         SSH public key to use (deprecated: use kops create secret instead)
       --target string                 Target - direct, terraform (default "direct")
       --user string                   Existing user in kubeconfig file to use.  Implies --create-kube-config

pkg/instancegroups/instancegroups.go

@@ -492,6 +492,8 @@ func (c *RollingUpdateCluster) reconcileInstanceGroup() error {
 		Phase:              "",
 		TargetName:         "direct",
 		LifecycleOverrides: map[string]fi.Lifecycle{},
+
+		DeletionProcessing: fi.DeletionProcessingModeDeleteIfNotDeferrred,
 	}
 
 	return applyCmd.Run(c.Ctx)

pkg/model/awsmodel/api_loadbalancer.go

@@ -124,8 +124,6 @@ func (b *APILoadBalancerBuilder) Build(c *fi.CloudupModelBuilderContext) error {
 	var clb *awstasks.ClassicLoadBalancer
 	var nlb *awstasks.NetworkLoadBalancer
 	{
-		loadBalancerName := b.LBName32("api")
-
 		idleTimeout := LoadBalancerDefaultIdleTimeout
 		if lbSpec.IdleTimeoutSeconds != nil {
 			idleTimeout = time.Second * time.Duration(*lbSpec.IdleTimeoutSeconds)
@@ -196,13 +194,12 @@ func (b *APILoadBalancerBuilder) Build(c *fi.CloudupModelBuilderContext) error {
 			Name:      fi.PtrTo(b.NLBName("api")),
 			Lifecycle: b.Lifecycle,
 
-			LoadBalancerName: fi.PtrTo(loadBalancerName),
+			LoadBalancerBaseName: fi.PtrTo(b.LBName32("api")),
 			CLBName:              fi.PtrTo("api." + b.ClusterName()),
 			SecurityGroups: []*awstasks.SecurityGroup{
 				b.LinkToELBSecurityGroup("api"),
 			},
 			SubnetMappings:    nlbSubnetMappings,
-
 			Tags:              tags,
 			WellKnownServices: []wellknownservices.WellKnownService{wellknownservices.KubeAPIServer},
 			VPC:               b.LinkToVPC(),
@@ -219,7 +216,7 @@ func (b *APILoadBalancerBuilder) Build(c *fi.CloudupModelBuilderContext) error {
 			Name:      fi.PtrTo("api." + b.ClusterName()),
 			Lifecycle: b.Lifecycle,
 
-			LoadBalancerName: fi.PtrTo(loadBalancerName),
+			LoadBalancerName: fi.PtrTo(b.LBName32("api")),
 			SecurityGroups: []*awstasks.SecurityGroup{
 				b.LinkToELBSecurityGroup("api"),
 			},
@@ -320,7 +317,7 @@ func (b *APILoadBalancerBuilder) Build(c *fi.CloudupModelBuilderContext) error {
 					UnhealthyThreshold: fi.PtrTo(int64(2)),
 					Shared:             fi.PtrTo(false),
 				}
-
+				tg.CreateNewRevisionsWith(nlb)
 				c.AddTask(tg)
 			}
 
@@ -344,6 +341,7 @@ func (b *APILoadBalancerBuilder) Build(c *fi.CloudupModelBuilderContext) error {
 					UnhealthyThreshold: fi.PtrTo(int64(2)),
 					Shared:             fi.PtrTo(false),
 				}
+				tg.CreateNewRevisionsWith(nlb)
 
 				c.AddTask(tg)
 			}
@@ -367,6 +365,7 @@ func (b *APILoadBalancerBuilder) Build(c *fi.CloudupModelBuilderContext) error {
 					UnhealthyThreshold: fi.PtrTo(int64(2)),
 					Shared:             fi.PtrTo(false),
 				}
+				secondaryTG.CreateNewRevisionsWith(nlb)
 				c.AddTask(secondaryTG)
 			}
 			for _, nlbListener := range nlbListeners {

pkg/model/awsmodel/autoscalinggroup.go

@@ -492,7 +492,7 @@ func (b *AutoscalingGroupModelBuilder) buildAutoScalingGroupTask(c *fi.CloudupMo
 		}
 
 		if extLB.TargetGroupARN != nil {
-			targetGroupName, err := awsup.GetTargetGroupNameFromARN(fi.ValueOf(extLB.TargetGroupARN))
+			targetGroupName, err := awsup.NameForExternalTargetGroup(fi.ValueOf(extLB.TargetGroupARN))
 			if err != nil {
 				return nil, err
 			}

pkg/model/awsmodel/bastion.go

@@ -319,8 +319,6 @@ func (b *BastionModelBuilder) Build(c *fi.CloudupModelBuilderContext) error {
 	// Create NLB itself
 	var nlb *awstasks.NetworkLoadBalancer
 	{
-		loadBalancerName := b.LBName32("bastion")
-
 		tags := b.CloudTags("", false)
 		for k, v := range b.Cluster.Spec.CloudLabels {
 			tags[k] = v
@@ -341,13 +339,12 @@ func (b *BastionModelBuilder) Build(c *fi.CloudupModelBuilderContext) error {
 			Name:      fi.PtrTo(b.NLBName("bastion")),
 			Lifecycle: b.Lifecycle,
 
-			LoadBalancerName: fi.PtrTo(loadBalancerName),
+			LoadBalancerBaseName: fi.PtrTo(b.LBName32("bastion")),
 			CLBName:              fi.PtrTo("bastion." + b.ClusterName()),
 			SubnetMappings:       nlbSubnetMappings,
 			SecurityGroups: []*awstasks.SecurityGroup{
 				b.LinkToELBSecurityGroup("bastion"),
 			},
-
 			Tags:          tags,
 			VPC:           b.LinkToVPC(),
 			Type:          fi.PtrTo("network"),
@@ -390,6 +387,7 @@ func (b *BastionModelBuilder) Build(c *fi.CloudupModelBuilderContext) error {
 			UnhealthyThreshold: fi.PtrTo(int64(2)),
 			Shared:             fi.PtrTo(false),
 		}
+		tg.CreateNewRevisionsWith(nlb)
 
 		c.AddTask(tg)
 

pkg/model/awsmodel/spotinst.go

@@ -851,7 +851,7 @@ func (b *SpotInstanceGroupModelBuilder) buildLoadBalancers(c *fi.CloudupModelBui
 			c.EnsureTask(lb)
 		}
 		if extLB.TargetGroupARN != nil {
-			targetGroupName, err := awsup.GetTargetGroupNameFromARN(fi.ValueOf(extLB.TargetGroupARN))
+			targetGroupName, err := awsup.NameForExternalTargetGroup(fi.ValueOf(extLB.TargetGroupARN))
 			if err != nil {
 				return nil, nil, err
 			}

pkg/resources/aws/aws.go

@@ -1624,7 +1624,7 @@ func ListTargetGroups(cloud fi.Cloud, vpcID, clusterName string) ([]*resources.R
 		id := aws.StringValue(tg.TargetGroupName)
 		resourceTracker := &resources.Resource{
 			Name:    id,
-			ID:      targetGroup.ARN(),
+			ID:      targetGroup.ARN,
 			Type:    TypeTargetGroup,
 			Deleter: DeleteTargetGroup,
 			Dumper:  DumpTargetGroup,

upup/pkg/fi/cloudup/apply_cluster.go

@@ -155,6 +155,9 @@ type ApplyClusterCmd struct {
 
 	// AdditionalObjects holds cluster-asssociated configuration objects, other than the Cluster and InstanceGroups.
 	AdditionalObjects kubemanifest.ObjectList
+
+	// DeletionProcessing controls whether we process deletions.
+	DeletionProcessing fi.DeletionProcessingMode
 }
 
 func (c *ApplyClusterCmd) Run(ctx context.Context) error {
@@ -714,7 +717,7 @@ func (c *ApplyClusterCmd) Run(ctx context.Context) error {
 	var target fi.CloudupTarget
 	shouldPrecreateDNS := true
 
-	deletionProcessingMode := fi.DeletionProcessingModeDeleteIfNotDeferrred
+	deletionProcessingMode := c.DeletionProcessing
 	switch c.TargetName {
 	case TargetDirect:
 		switch cluster.Spec.GetCloudProvider() {

upup/pkg/fi/cloudup/awstasks/autoscalinggroup.go

@@ -101,8 +101,11 @@ type AutoscalingGroup struct {
 	CapacityRebalance *bool
 	// WarmPool is the WarmPool config for the ASG
 	WarmPool *WarmPool
+
+	deletions []fi.CloudupDeletion
 }
 
+var _ fi.CloudupProducesDeletions = &AutoscalingGroup{}
 var _ fi.CompareWithID = &AutoscalingGroup{}
 var _ fi.CloudupTaskNormalize = &AutoscalingGroup{}
 
@@ -212,13 +215,21 @@ func (e *AutoscalingGroup) Find(c *fi.CloudupContext) (*AutoscalingGroup, error)
 	sort.Stable(OrderLoadBalancersByName(actual.LoadBalancers))
 
 	actual.TargetGroups = []*TargetGroup{}
-	if len(g.TargetGroupARNs) > 0 {
+	{
-		for _, tg := range g.TargetGroupARNs {
+		byARN := make(map[string]*TargetGroup)
-			targetGroupName, err := awsup.GetTargetGroupNameFromARN(fi.ValueOf(tg))
+		for _, tg := range e.TargetGroups {
-			if err != nil {
+			if tg.info != nil {
-				return nil, err
+				byARN[tg.info.ARN] = tg
 			}
-			actual.TargetGroups = append(actual.TargetGroups, &TargetGroup{ARN: aws.String(*tg), Name: aws.String(targetGroupName)})
+		}
+		for _, arn := range g.TargetGroupARNs {
+			tg := byARN[aws.StringValue(arn)]
+			if tg != nil {
+				actual.TargetGroups = append(actual.TargetGroups, tg)
+				continue
+			}
+			actual.TargetGroups = append(actual.TargetGroups, &TargetGroup{ARN: arn})
+			e.deletions = append(e.deletions, buildDeleteAutoscalingTargetGroupAttachment(aws.StringValue(g.AutoScalingGroupName), aws.StringValue(arn)))
 		}
 	}
 	sort.Stable(OrderTargetGroupsByName(actual.TargetGroups))
@@ -612,7 +623,6 @@ func (v *AutoscalingGroup) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *Autos
 		}
 
 		var attachTGRequests []*autoscaling.AttachLoadBalancerTargetGroupsInput
-		var detachTGRequests []*autoscaling.DetachLoadBalancerTargetGroupsInput
 		if changes.TargetGroups != nil {
 			if e != nil && len(e.TargetGroups) > 0 {
 				for _, tgsChunkToAttach := range sliceChunks(e.AutoscalingTargetGroups(), attachLoadBalancerTargetGroupsMaxItems) {
@@ -623,14 +633,8 @@ func (v *AutoscalingGroup) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *Autos
 				}
 			}
 
-			if a != nil && len(a.TargetGroups) > 0 {
+			// Detaching is done in a deletion task
-				for _, tgsChunkToDetach := range sliceChunks(e.getTGsToDetach(a.TargetGroups), detachLoadBalancerTargetGroupsMaxItems) {
+
-					detachTGRequests = append(detachTGRequests, &autoscaling.DetachLoadBalancerTargetGroupsInput{
-						AutoScalingGroupName: e.Name,
-						TargetGroupARNs:      tgsChunkToDetach,
-					})
-				}
-			}
 			changes.TargetGroups = nil
 		}
 
@@ -719,13 +723,6 @@ func (v *AutoscalingGroup) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *Autos
 				return fmt.Errorf("error attaching LoadBalancers: %v", err)
 			}
 		}
-		if len(detachTGRequests) > 0 {
-			for _, detachTGRequest := range detachTGRequests {
-				if _, err := t.Cloud.Autoscaling().DetachLoadBalancerTargetGroupsWithContext(ctx, detachTGRequest); err != nil {
-					return fmt.Errorf("failed to detach target groups: %v", err)
-				}
-			}
-		}
 		if len(attachTGRequests) > 0 {
 			for _, attachTGRequest := range attachTGRequests {
 				if _, err := t.Cloud.Autoscaling().AttachLoadBalancerTargetGroupsWithContext(ctx, attachTGRequest); err != nil {
@@ -1121,3 +1118,52 @@ func (_ *AutoscalingGroup) RenderTerraform(t *terraform.TerraformTarget, a, e, c
 func (e *AutoscalingGroup) TerraformLink() *terraformWriter.Literal {
 	return terraformWriter.LiteralProperty("aws_autoscaling_group", fi.ValueOf(e.Name), "id")
 }
+
+func (e *AutoscalingGroup) FindDeletions(context *fi.CloudupContext) ([]fi.CloudupDeletion, error) {
+	return e.deletions, nil
+}
+
+type deleteAutoscalingTargetGroupAttachment struct {
+	autoScalingGroupName string
+	targetGroupARN       string
+}
+
+var _ fi.CloudupDeletion = &deleteAutoscalingTargetGroupAttachment{}
+
+func buildDeleteAutoscalingTargetGroupAttachment(autoScalingGroupName string, targetGroupARN string) *deleteAutoscalingTargetGroupAttachment {
+	d := &deleteAutoscalingTargetGroupAttachment{}
+	d.autoScalingGroupName = autoScalingGroupName
+	d.targetGroupARN = targetGroupARN
+	return d
+}
+
+func (d *deleteAutoscalingTargetGroupAttachment) Delete(t fi.CloudupTarget) error {
+	ctx := context.TODO()
+
+	awsTarget, ok := t.(*awsup.AWSAPITarget)
+	if !ok {
+		return fmt.Errorf("unexpected target type for deletion: %T", t)
+	}
+
+	req := &autoscaling.DetachLoadBalancerTargetGroupsInput{
+		AutoScalingGroupName: aws.String(d.autoScalingGroupName),
+		TargetGroupARNs:      aws.StringSlice([]string{d.targetGroupARN}),
+	}
+	if _, err := awsTarget.Cloud.Autoscaling().DetachLoadBalancerTargetGroupsWithContext(ctx, req); err != nil {
+		return fmt.Errorf("failed to detach target groups from autoscaling group: %v", err)
+	}
+
+	return nil
+}
+
+func (d *deleteAutoscalingTargetGroupAttachment) TaskName() string {
+	return "autoscaling-elb-attachment"
+
+}
+func (d *deleteAutoscalingTargetGroupAttachment) Item() string {
+	return d.autoScalingGroupName + ":" + d.targetGroupARN
+}
+
+func (d *deleteAutoscalingTargetGroupAttachment) DeferDeletion() bool {
+	return true
+}

upup/pkg/fi/cloudup/awstasks/dnsname.go

@@ -50,6 +50,7 @@ type DNSTarget interface {
 }
 
 func (e *DNSName) Find(c *fi.CloudupContext) (*DNSName, error) {
+	ctx := c.Context()
 	cloud := c.T.Cloud.(awsup.AWSCloud)
 
 	if e.Zone == nil || e.Zone.ZoneID == nil {
@@ -75,7 +76,7 @@ func (e *DNSName) Find(c *fi.CloudupContext) (*DNSName, error) {
 
 	var found *route53.ResourceRecordSet
 
-	err := cloud.Route53().ListResourceRecordSetsPages(request, func(p *route53.ListResourceRecordSetsOutput, lastPage bool) (shouldContinue bool) {
+	err := cloud.Route53().ListResourceRecordSetsPagesWithContext(ctx, request, func(p *route53.ListResourceRecordSetsOutput, lastPage bool) (shouldContinue bool) {
 		for _, rr := range p.ResourceRecordSets {
 			resourceType := aws.StringValue(rr.Type)
 			name := aws.StringValue(rr.Name)

upup/pkg/fi/cloudup/awstasks/launchtemplate_target_api.go

@@ -402,7 +402,7 @@ func (t *LaunchTemplate) findLatestLaunchTemplateVersion(c *fi.CloudupContext) (
 }
 
 // deleteLaunchTemplate tracks a LaunchConfiguration that we're going to delete
-// It implements fi.Deletion
+// It implements fi.CloudupDeletion
 type deleteLaunchTemplate struct {
 	lc *ec2.LaunchTemplate
 }
@@ -438,3 +438,7 @@ func (d *deleteLaunchTemplate) Delete(t fi.CloudupTarget) error {
 func (d *deleteLaunchTemplate) String() string {
 	return d.TaskName() + "-" + d.Item()
 }
+
+func (d *deleteLaunchTemplate) DeferDeletion() bool {
+	return false // TODO: Should we defer deletion?
+}

upup/pkg/fi/cloudup/awstasks/network_load_balancer.go

@@ -22,13 +22,14 @@ import (
 	"sort"
 	"strconv"
 	"strings"
+	"time"
 
 	"github.com/aws/aws-sdk-go/aws"
-	"github.com/aws/aws-sdk-go/aws/awserr"
 	"github.com/aws/aws-sdk-go/service/elb"
 	"github.com/aws/aws-sdk-go/service/elbv2"
 	"github.com/aws/aws-sdk-go/service/route53"
 	"k8s.io/klog/v2"
+	"k8s.io/kops/pkg/truncate"
 	"k8s.io/kops/pkg/wellknownservices"
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/upup/pkg/fi/cloudup/awsup"
@@ -46,10 +47,13 @@ type NetworkLoadBalancer struct {
 	Name      *string
 	Lifecycle fi.Lifecycle
 
-	// LoadBalancerName is the name in NLB, possibly different from our name
+	// LoadBalancerBaseName is the base name to use when naming load balancers in NLB.
+	// The full, stable name will be in the Name tag.
 	// (NLB is restricted as to names, so we have limited choices!)
-	// We use the Name tag to find the existing NLB.
+	LoadBalancerBaseName *string
-	LoadBalancerName *string
+
+	// CLBName is the name of a ClassicLoadBalancer to delete, if found.
+	// This enables migration from CLB -> NLB
 	CLBName *string
 
 	DNSName      *string
@@ -80,6 +84,12 @@ type NetworkLoadBalancer struct {
 
 	// After this is found/created, we store the ARN
 	loadBalancerArn string
+
+	// After this is found/created, we store the revision
+	revision string
+
+	// deletions is a list of previous versions of this object, that we should delete when asked to clean up.
+	deletions []fi.CloudupDeletion
 }
 
 func (e *NetworkLoadBalancer) SetWaitForLoadBalancerReady(v bool) {
@@ -94,43 +104,9 @@ func (e *NetworkLoadBalancer) CompareWithID() *string {
 	return e.Name
 }
 
-// The load balancer name 'api.renamenlbcluster.k8s.local' can only contain characters that are alphanumeric characters and hyphens(-)\n\tstatus code: 400,
-func findNetworkLoadBalancerByLoadBalancerName(cloud awsup.AWSCloud, loadBalancerName string) (*elbv2.LoadBalancer, error) {
-	request := &elbv2.DescribeLoadBalancersInput{
-		Names: []*string{&loadBalancerName},
-	}
-	found, err := describeNetworkLoadBalancers(cloud, request, func(lb *elbv2.LoadBalancer) bool {
-		// TODO: Filter by cluster?
-
-		if aws.StringValue(lb.LoadBalancerName) == loadBalancerName {
-			return true
-		}
-
-		klog.Warningf("Got NLB with unexpected name: %q", aws.StringValue(lb.LoadBalancerName))
-		return false
-	})
-	if err != nil {
-		if awsError, ok := err.(awserr.Error); ok {
-			if awsError.Code() == "LoadBalancerNotFound" {
-				return nil, nil
-			}
-		}
-
-		return nil, fmt.Errorf("error listing NLBs: %v", err)
-	}
-
-	if len(found) == 0 {
-		return nil, nil
-	}
-
-	if len(found) != 1 {
-		return nil, fmt.Errorf("Found multiple NLBs with name %q", loadBalancerName)
-	}
-
-	return found[0], nil
-}
-
 func findNetworkLoadBalancerByAlias(cloud awsup.AWSCloud, alias *route53.AliasTarget) (*elbv2.LoadBalancer, error) {
+	ctx := context.TODO()
+
 	// TODO: Any way to avoid listing all NLBs?
 	request := &elbv2.DescribeLoadBalancersInput{}
 
@@ -142,9 +118,7 @@ func findNetworkLoadBalancerByAlias(cloud awsup.AWSCloud, alias *route53.AliasTa
 
 	matchHostedZoneId := aws.StringValue(alias.HostedZoneId)
 
-	found, err := describeNetworkLoadBalancers(cloud, request, func(lb *elbv2.LoadBalancer) bool {
+	found, err := describeNetworkLoadBalancers(ctx, cloud, request, func(lb *elbv2.LoadBalancer) bool {
-		// TODO: Filter by cluster?
-
 		if matchHostedZoneId != aws.StringValue(lb.CanonicalHostedZoneId) {
 			return false
 		}
@@ -168,9 +142,9 @@ func findNetworkLoadBalancerByAlias(cloud awsup.AWSCloud, alias *route53.AliasTa
 	return found[0], nil
 }
 
-func describeNetworkLoadBalancers(cloud awsup.AWSCloud, request *elbv2.DescribeLoadBalancersInput, filter func(*elbv2.LoadBalancer) bool) ([]*elbv2.LoadBalancer, error) {
+func describeNetworkLoadBalancers(ctx context.Context, cloud awsup.AWSCloud, request *elbv2.DescribeLoadBalancersInput, filter func(*elbv2.LoadBalancer) bool) ([]*elbv2.LoadBalancer, error) {
 	var found []*elbv2.LoadBalancer
-	err := cloud.ELBV2().DescribeLoadBalancersPages(request, func(p *elbv2.DescribeLoadBalancersOutput, lastPage bool) (shouldContinue bool) {
+	err := cloud.ELBV2().DescribeLoadBalancersPagesWithContext(ctx, request, func(p *elbv2.DescribeLoadBalancersOutput, lastPage bool) (shouldContinue bool) {
 		for _, lb := range p.LoadBalancers {
 			if filter(lb) {
 				found = append(found, lb)
@@ -195,22 +169,44 @@ func (e *NetworkLoadBalancer) getHostedZoneId() *string {
 }
 
 func (e *NetworkLoadBalancer) Find(c *fi.CloudupContext) (*NetworkLoadBalancer, error) {
+	ctx := c.Context()
 	cloud := c.T.Cloud.(awsup.AWSCloud)
 
-	lb, err := cloud.FindELBV2ByNameTag(e.Tags["Name"])
+	allLoadBalancers, err := awsup.ListELBV2LoadBalancers(ctx, cloud)
 	if err != nil {
 		return nil, err
 	}
-	if lb == nil {
+
+	latest := awsup.FindLatestELBV2ByNameTag(allLoadBalancers, fi.ValueOf(e.Name))
+	if err != nil {
+		return nil, err
+	}
+
+	// Stash deletions for later
+	for _, lb := range allLoadBalancers {
+		if lb.NameTag() != fi.ValueOf(e.Name) {
+			continue
+		}
+		if latest != nil && latest.ARN() == lb.ARN() {
+			continue
+		}
+
+		e.deletions = append(e.deletions, &deleteNLB{
+			obj: lb,
+		})
+	}
+
+	if latest == nil {
 		return nil, nil
 	}
 
-	loadBalancerArn := lb.LoadBalancerArn
+	lb := latest.LoadBalancer
+
+	loadBalancerArn := latest.ARN()
 
 	actual := &NetworkLoadBalancer{}
 	actual.Name = e.Name
 	actual.CLBName = e.CLBName
-	actual.LoadBalancerName = lb.LoadBalancerName
 	actual.DNSName = lb.DNSName
 	actual.HostedZoneId = lb.CanonicalHostedZoneId // CanonicalHostedZoneNameID
 	actual.Scheme = lb.Scheme
@@ -218,16 +214,16 @@ func (e *NetworkLoadBalancer) Find(c *fi.CloudupContext) (*NetworkLoadBalancer,
 	actual.Type = lb.Type
 	actual.IpAddressType = lb.IpAddressType
 
-	tagMap, err := cloud.DescribeELBV2Tags([]string{*loadBalancerArn})
-	if err != nil {
-		return nil, err
-	}
 	actual.Tags = make(map[string]string)
-	for _, tag := range tagMap[*loadBalancerArn] {
+	for _, tag := range latest.Tags {
-		if strings.HasPrefix(aws.StringValue(tag.Key), "aws:cloudformation:") {
+		k := aws.StringValue(tag.Key)
+		if strings.HasPrefix(k, "aws:cloudformation:") {
 			continue
 		}
-		actual.Tags[aws.StringValue(tag.Key)] = aws.StringValue(tag.Value)
+		if k == awsup.KopsResourceRevisionTag {
+			continue
+		}
+		actual.Tags[k] = aws.StringValue(tag.Value)
 	}
 
 	for _, az := range lb.AvailabilityZones {
@@ -256,7 +252,7 @@ func (e *NetworkLoadBalancer) Find(c *fi.CloudupContext) (*NetworkLoadBalancer,
 	}
 
 	{
-		lbAttributes, err := findNetworkLoadBalancerAttributes(cloud, aws.StringValue(loadBalancerArn))
+		lbAttributes, err := findNetworkLoadBalancerAttributes(cloud, loadBalancerArn)
 		if err != nil {
 			return nil, err
 		}
@@ -312,31 +308,35 @@ func (e *NetworkLoadBalancer) Find(c *fi.CloudupContext) (*NetworkLoadBalancer,
 	if e.HostedZoneId == nil {
 		e.HostedZoneId = actual.HostedZoneId
 	}
-	if e.LoadBalancerName == nil {
-		e.LoadBalancerName = actual.LoadBalancerName
-	}
 
 	// An existing internal NLB can't be updated to dualstack.
 	if fi.ValueOf(actual.Scheme) == elbv2.LoadBalancerSchemeEnumInternal && fi.ValueOf(actual.IpAddressType) == elbv2.IpAddressTypeIpv4 {
 		e.IpAddressType = actual.IpAddressType
 	}
 
-	// We allow for the LoadBalancerName to be wrong:
-	// 1. We don't want to force a rename of the NLB, because that is a destructive operation
-	if fi.ValueOf(e.LoadBalancerName) != fi.ValueOf(actual.LoadBalancerName) {
-		klog.V(2).Infof("Reusing existing load balancer with name: %q", aws.StringValue(actual.LoadBalancerName))
-		e.LoadBalancerName = actual.LoadBalancerName
-	}
-
 	_ = actual.Normalize(c)
 	actual.WellKnownServices = e.WellKnownServices
 	actual.Lifecycle = e.Lifecycle
+	actual.LoadBalancerBaseName = e.LoadBalancerBaseName
 
-	// Store for other tasks
+	// Store state for other tasks
 	e.loadBalancerArn = aws.StringValue(lb.LoadBalancerArn)
+	actual.loadBalancerArn = e.loadBalancerArn
+	e.revision, _ = latest.GetTag(awsup.KopsResourceRevisionTag)
+	actual.revision = e.revision
 
 	klog.V(4).Infof("Found NLB %+v", actual)
 
+	// AWS does not allow us to add security groups to an ELB that was initially created without them.
+	// This forces a new revision (currently, the only operation that forces a new revision)
+	if len(actual.SecurityGroups) == 0 && len(e.SecurityGroups) > 0 {
+		klog.Warningf("setting securityGroups on an existing NLB created without securityGroups; will force a new NLB")
+		t := time.Now()
+		revision := strconv.FormatInt(t.Unix(), 10)
+		actual = nil
+		e.revision = revision
+	}
+
 	return actual, nil
 }
 
@@ -348,26 +348,31 @@ func (e *NetworkLoadBalancer) GetWellKnownServices() []wellknownservices.WellKno
 	return e.WellKnownServices
 }
 
-func (e *NetworkLoadBalancer) FindAddresses(context *fi.CloudupContext) ([]string, error) {
+func (e *NetworkLoadBalancer) FindAddresses(c *fi.CloudupContext) ([]string, error) {
+	ctx := c.Context()
+
 	var addresses []string
 
-	cloud := context.T.Cloud.(awsup.AWSCloud)
+	cloud := c.T.Cloud.(awsup.AWSCloud)
-	cluster := context.T.Cluster
+	cluster := c.T.Cluster
 
 	{
-		lb, err := cloud.FindELBV2ByNameTag(e.Tags["Name"])
+		allLoadBalancers, err := awsup.ListELBV2LoadBalancers(ctx, cloud)
 		if err != nil {
-			return nil, fmt.Errorf("failed to find load balancer matching %q: %w", e.Tags["Name"], err)
+			return nil, err
-		}
-		if lb != nil && fi.ValueOf(lb.DNSName) != "" {
-			addresses = append(addresses, fi.ValueOf(lb.DNSName))
 		}
+
+		lb := awsup.FindLatestELBV2ByNameTag(allLoadBalancers, fi.ValueOf(e.Name))
+
+		if lb != nil {
+			if fi.ValueOf(lb.LoadBalancer.DNSName) != "" {
+				addresses = append(addresses, fi.ValueOf(lb.LoadBalancer.DNSName))
 			}
 
 			if cluster.UsesNoneDNS() {
-		nis, err := cloud.FindELBV2NetworkInterfacesByName(fi.ValueOf(e.VPC.ID), fi.ValueOf(e.LoadBalancerName))
+				nis, err := cloud.FindELBV2NetworkInterfacesByName(fi.ValueOf(e.VPC.ID), aws.StringValue(lb.LoadBalancer.LoadBalancerName))
 				if err != nil {
-			return nil, fmt.Errorf("failed to find network interfaces matching %q: %w", fi.ValueOf(e.LoadBalancerName), err)
+					return nil, fmt.Errorf("failed to find network interfaces matching %q: %w", aws.StringValue(lb.LoadBalancer.LoadBalancerName), err)
 				}
 				for _, ni := range nis {
 					if fi.ValueOf(ni.PrivateIpAddress) != "" {
@@ -375,6 +380,8 @@ func (e *NetworkLoadBalancer) FindAddresses(context *fi.CloudupContext) ([]strin
 					}
 				}
 			}
+		}
+	}
 
 	sort.Strings(addresses)
 
@@ -456,23 +463,42 @@ func (*NetworkLoadBalancer) CheckChanges(a, e, changes *NetworkLoadBalancer) err
 func (_ *NetworkLoadBalancer) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *NetworkLoadBalancer) error {
 	ctx := context.TODO()
 
-	var loadBalancerName string
+	loadBalancerArn := ""
-	var loadBalancerArn string
 
-	if a == nil {
+	revision := e.revision
-		if e.LoadBalancerName == nil {
+
-			return fi.RequiredField("LoadBalancerName")
+	// TODO: Use maps.Clone when we are >= go1.21 on supported branches
+	tags := make(map[string]string)
+	for k, v := range e.Tags {
+		tags[k] = v
 	}
 
-		loadBalancerName = *e.LoadBalancerName
+	// We removed revision for the diff/plan, but we want to set it
+	if revision != "" {
+		tags[awsup.KopsResourceRevisionTag] = revision
+	}
+
+	if a == nil {
+		loadBalancerName := fi.ValueOf(e.LoadBalancerBaseName)
+		if revision != "" {
+			s := fi.ValueOf(e.LoadBalancerBaseName) + "-" + revision
+
+			// We always compute the hash and add it, lest we trick users into assuming that we never do this
+			opt := truncate.TruncateStringOptions{
+				MaxLength:     32,
+				AlwaysAddHash: true,
+				HashLength:    6,
+			}
+			loadBalancerName = truncate.TruncateString(s, opt)
+		}
 
 		{
 			request := &elbv2.CreateLoadBalancerInput{}
-			request.Name = e.LoadBalancerName
+			request.Name = &loadBalancerName
 			request.Scheme = e.Scheme
 			request.Type = e.Type
 			request.IpAddressType = e.IpAddressType
-			request.Tags = awsup.ELBv2Tags(e.Tags)
+			request.Tags = awsup.ELBv2Tags(tags)
 
 			for _, subnetMapping := range e.SubnetMappings {
 				request.SubnetMappings = append(request.SubnetMappings, &elbv2.SubnetMapping{
@@ -502,6 +528,7 @@ func (_ *NetworkLoadBalancer) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *Ne
 			e.VPC = &VPC{ID: lb.VpcId}
 			loadBalancerArn = aws.StringValue(lb.LoadBalancerArn)
 			e.loadBalancerArn = loadBalancerArn
+			e.revision = revision
 		}
 
 		if e.waitForLoadBalancerReady {
@@ -517,19 +544,12 @@ func (_ *NetworkLoadBalancer) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *Ne
 		}
 
 	} else {
-		loadBalancerName = fi.ValueOf(a.LoadBalancerName)
+		loadBalancerArn = a.loadBalancerArn
-
-		lb, err := findNetworkLoadBalancerByLoadBalancerName(t.Cloud, loadBalancerName)
-		if err != nil {
-			return fmt.Errorf("error getting load balancer by name: %v", err)
-		}
-
-		loadBalancerArn = fi.ValueOf(lb.LoadBalancerArn)
 
 		if changes.IpAddressType != nil {
 			request := &elbv2.SetIpAddressTypeInput{
 				IpAddressType:   e.IpAddressType,
-				LoadBalancerArn: lb.LoadBalancerArn,
+				LoadBalancerArn: &loadBalancerArn,
 			}
 			if _, err := t.Cloud.ELBV2().SetIpAddressType(request); err != nil {
 				return fmt.Errorf("error setting the IP addresses type: %v", err)
@@ -576,7 +596,7 @@ func (_ *NetworkLoadBalancer) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *Ne
 
 		if changes.SecurityGroups != nil {
 			request := &elbv2.SetSecurityGroupsInput{
-				LoadBalancerArn: lb.LoadBalancerArn,
+				LoadBalancerArn: &loadBalancerArn,
 			}
 			for _, sg := range e.SecurityGroups {
 				request.SecurityGroups = append(request.SecurityGroups, sg.ID)
@@ -588,11 +608,11 @@ func (_ *NetworkLoadBalancer) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *Ne
 			}
 		}
 
-		if err := t.AddELBV2Tags(loadBalancerArn, e.Tags); err != nil {
+		if err := t.AddELBV2Tags(loadBalancerArn, tags); err != nil {
 			return err
 		}
 
-		if err := t.RemoveELBV2Tags(loadBalancerArn, e.Tags); err != nil {
+		if err := t.RemoveELBV2Tags(loadBalancerArn, tags); err != nil {
 			return err
 		}
 	}
@@ -625,7 +645,7 @@ type terraformNetworkLoadBalancerSubnetMapping struct {
 
 func (_ *NetworkLoadBalancer) RenderTerraform(t *terraform.TerraformTarget, a, e, changes *NetworkLoadBalancer) error {
 	nlbTF := &terraformNetworkLoadBalancer{
-		Name:                   *e.LoadBalancerName,
+		Name:                   *e.LoadBalancerBaseName,
 		Internal:               fi.ValueOf(e.Scheme) == elbv2.LoadBalancerSchemeEnumInternal,
 		Type:                   elbv2.LoadBalancerTypeEnumNetwork,
 		Tags:                   e.Tags,
@@ -679,33 +699,25 @@ func (e *NetworkLoadBalancer) TerraformLink(params ...string) *terraformWriter.L
 
 // FindDeletions schedules deletion of the corresponding legacy classic load balancer when it no longer has targets.
 func (e *NetworkLoadBalancer) FindDeletions(context *fi.CloudupContext) ([]fi.CloudupDeletion, error) {
-	if e.CLBName == nil {
+	var deletions []fi.CloudupDeletion
-		return nil, nil
-	}
 
+	deletions = append(deletions, e.deletions...)
+
+	if e.CLBName != nil {
 		cloud := context.T.Cloud.(awsup.AWSCloud)
 
 		lb, err := cloud.FindELBByNameTag(fi.ValueOf(e.CLBName))
 		if err != nil {
 			return nil, err
 		}
-	if lb == nil {
+
-		return nil, nil
+		if lb != nil {
+			klog.V(4).Infof("Found CLB %v", aws.StringValue(lb.LoadBalancerName))
+			deletions = append(deletions, &deleteClassicLoadBalancer{LoadBalancerName: e.CLBName})
+		}
 	}
 
-	// Testing shows that the instances are deregistered immediately after the apply_cluster.
+	return deletions, nil
-	// TODO: Figure out how to delay deregistration until instances are terminated.
-	//if len(lb.Instances) > 0 {
-	//	klog.V(2).Infof("CLB %s has targets; not scheduling deletion", *lb.LoadBalancerName)
-	//	return nil, nil
-	//}
-
-	actual := &deleteClassicLoadBalancer{}
-	actual.LoadBalancerName = lb.LoadBalancerName
-
-	klog.V(4).Infof("Found CLB %+v", actual)
-
-	return []fi.CloudupDeletion{actual}, nil
 }
 
 type deleteClassicLoadBalancer struct {
@@ -714,6 +726,18 @@ type deleteClassicLoadBalancer struct {
 	LoadBalancerName *string
 }
 
+func (d deleteClassicLoadBalancer) TaskName() string {
+	return "ClassicLoadBalancer"
+}
+
+func (d deleteClassicLoadBalancer) Item() string {
+	return *d.LoadBalancerName
+}
+
+func (d deleteClassicLoadBalancer) DeferDeletion() bool {
+	return true
+}
+
 func (d deleteClassicLoadBalancer) Delete(t fi.CloudupTarget) error {
 	awsTarget, ok := t.(*awsup.AWSAPITarget)
 	if !ok {
@@ -730,10 +754,54 @@ func (d deleteClassicLoadBalancer) Delete(t fi.CloudupTarget) error {
 	return nil
 }
 
-func (d deleteClassicLoadBalancer) TaskName() string {
+// deleteNLB tracks a NLB that we're going to delete
-	return "ClassicLoadBalancer"
+// It implements fi.CloudupDeletion
+type deleteNLB struct {
+	obj *awsup.LoadBalancerInfo
 }
 
-func (d deleteClassicLoadBalancer) Item() string {
+func buildDeleteNLB(obj *awsup.LoadBalancerInfo) *deleteNLB {
-	return *d.LoadBalancerName
+	d := &deleteNLB{}
+	d.obj = obj
+	return d
+}
+
+var _ fi.CloudupDeletion = &deleteNLB{}
+
+func (d *deleteNLB) Delete(t fi.CloudupTarget) error {
+	ctx := context.TODO()
+
+	awsTarget, ok := t.(*awsup.AWSAPITarget)
+	if !ok {
+		return fmt.Errorf("unexpected target type for deletion: %T", t)
+	}
+
+	arn := d.obj.ARN()
+	klog.V(2).Infof("deleting load balancer %q", arn)
+	if _, err := awsTarget.Cloud.ELBV2().DeleteLoadBalancerWithContext(ctx, &elbv2.DeleteLoadBalancerInput{
+		LoadBalancerArn: &arn,
+	}); err != nil {
+		return fmt.Errorf("error deleting ELB LoadBalancer %q: %w", arn, err)
+	}
+
+	return nil
+}
+
+// String returns a string representation of the task
+func (d *deleteNLB) String() string {
+	return d.TaskName() + "-" + d.Item()
+}
+
+// TaskName returns the task name
+func (d *deleteNLB) TaskName() string {
+	return "network-load-balancer"
+}
+
+// Item returns the launch template name
+func (d *deleteNLB) Item() string {
+	return d.obj.ARN()
+}
+
+func (d *deleteNLB) DeferDeletion() bool {
+	return true
 }

upup/pkg/fi/cloudup/awstasks/networkloadbalancer_attributes.go

@@ -72,8 +72,6 @@ func (_ *NetworkLoadBalancer) modifyLoadBalancerAttributes(t *awsup.AWSAPITarget
 		return nil
 	}
 
-	loadBalancerName := fi.ValueOf(e.LoadBalancerName)
-
 	request := &elbv2.ModifyLoadBalancerAttributesInput{
 		LoadBalancerArn: aws.String(loadBalancerArn),
 	}
@@ -113,14 +111,14 @@ func (_ *NetworkLoadBalancer) modifyLoadBalancerAttributes(t *awsup.AWSAPITarget
 
 	request.Attributes = attributes
 
-	klog.V(2).Infof("Configuring NLB attributes for NLB %q", loadBalancerName)
+	klog.V(2).Infof("Configuring NLB attributes for NLB %q", loadBalancerArn)
 
 	response, err := t.Cloud.ELBV2().ModifyLoadBalancerAttributes(request)
 	if err != nil {
-		return fmt.Errorf("error configuring NLB attributes for NLB %q: %v", loadBalancerName, err)
+		return fmt.Errorf("error configuring NLB attributes for NLB %q: %v", loadBalancerArn, err)
 	}
 
-	klog.V(4).Infof("modified NLB attributes for NLB %q, response %+v", loadBalancerName, response)
+	klog.V(4).Infof("modified NLB attributes for NLB %q, response %+v", loadBalancerArn, response)
 
 	return nil
 }

upup/pkg/fi/cloudup/awstasks/networkloadbalancerlistener.go

@@ -31,6 +31,8 @@ import (
 
 // +kops:fitask
 type NetworkLoadBalancerListener struct {
+	// We use the Name tag to find the existing NLB, because we are (more or less) unrestricted when
+	// it comes to tag values, but the LoadBalancerName is length limited
 	Name      *string
 	Lifecycle fi.Lifecycle
 
@@ -203,8 +205,6 @@ func (*NetworkLoadBalancerListener) RenderAWS(t *awsup.AWSAPITarget, a, e, chang
 		}
 	}
 
-	// TODO: Tags on the listener?
-
 	return nil
 }
 
@@ -226,7 +226,6 @@ func (_ *NetworkLoadBalancerListener) RenderTerraform(t *terraform.TerraformTarg
 	if e.TargetGroup == nil {
 		return fi.RequiredField("TargetGroup")
 	}
-
 	listenerTF := &terraformNetworkLoadBalancerListener{
 		LoadBalancer: e.NetworkLoadBalancer.TerraformLink(),
 		Port:         int64(e.Port),

upup/pkg/fi/cloudup/awstasks/securitygroup.go

@@ -222,10 +222,18 @@ func (e *SecurityGroup) TerraformLink() *terraformWriter.Literal {
 	return terraformWriter.LiteralProperty("aws_security_group", *e.Name, "id")
 }
 
+// deleteSecurityGroupRule tracks a securitygrouprule that we're going to delete
+// It implements fi.CloudupDeletion
 type deleteSecurityGroupRule struct {
 	rule *ec2.SecurityGroupRule
 }
 
+func buildDeleteSecurityGroupRule(rule *ec2.SecurityGroupRule) *deleteSecurityGroupRule {
+	d := &deleteSecurityGroupRule{}
+	d.rule = rule
+	return d
+}
+
 var _ fi.CloudupDeletion = &deleteSecurityGroupRule{}
 
 func (d *deleteSecurityGroupRule) Delete(t fi.CloudupTarget) error {
@@ -236,7 +244,7 @@ func (d *deleteSecurityGroupRule) Delete(t fi.CloudupTarget) error {
 		return fmt.Errorf("unexpected target type for deletion: %T", t)
 	}
 
-	if fi.ValueOf(d.rule.IsEgress) {
+	if aws.BoolValue(d.rule.IsEgress) {
 		request := &ec2.RevokeSecurityGroupEgressInput{
 			GroupId:              d.rule.GroupId,
 			SecurityGroupRuleIds: []*string{d.rule.SecurityGroupRuleId},
@@ -290,6 +298,10 @@ func (d *deleteSecurityGroupRule) Item() string {
 	return s
 }
 
+func (d *deleteSecurityGroupRule) DeferDeletion() bool {
+	return true
+}
+
 func (e *SecurityGroup) FindDeletions(c *fi.CloudupContext) ([]fi.CloudupDeletion, error) {
 	var removals []fi.CloudupDeletion
 
@@ -360,9 +372,7 @@ func (e *SecurityGroup) FindDeletions(c *fi.CloudupContext) ([]fi.CloudupDeletio
 			}
 		}
 		if !found {
-			removals = append(removals, &deleteSecurityGroupRule{
+			removals = append(removals, buildDeleteSecurityGroupRule(permission))
-				rule: permission,
-			})
 		}
 	}
 

upup/pkg/fi/cloudup/awstasks/subnet.go

@@ -541,6 +541,10 @@ func (d *deleteSubnetIPv6CIDRBlock) Item() string {
 	return fmt.Sprintf("%v: ipv6cidr=%v", *d.vpcID, *d.ipv6CidrBlock)
 }
 
+func (d *deleteSubnetIPv6CIDRBlock) DeferDeletion() bool {
+	return false // TODO: should we defer this?
+}
+
 func calculateSubnetCIDR(vpcCIDR, subnetCIDR *string) (*string, error) {
 	if vpcCIDR == nil {
 		return nil, fmt.Errorf("expecting VPC CIDR to not be <nil>")

upup/pkg/fi/cloudup/awstasks/targetgroup.go

@@ -19,11 +19,13 @@ package awstasks
 import (
 	"context"
 	"fmt"
+	"strconv"
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/aws/awserr"
 	"github.com/aws/aws-sdk-go/service/elbv2"
 	"k8s.io/klog/v2"
+	"k8s.io/kops/pkg/truncate"
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/upup/pkg/fi/cloudup/awsup"
 	"k8s.io/kops/upup/pkg/fi/cloudup/terraform"
@@ -50,6 +52,9 @@ type TargetGroup struct {
 	Port      *int64
 	Protocol  *string
 
+	// networkLoadBalancer, if set, will create a new Target Group for each revision of the Network Load Balancer
+	networkLoadBalancer *NetworkLoadBalancer
+
 	// ARN is the Amazon Resource Name for the Target Group
 	ARN *string
 
@@ -61,15 +66,41 @@ type TargetGroup struct {
 	Interval           *int64
 	HealthyThreshold   *int64
 	UnhealthyThreshold *int64
+
+	info     *awsup.TargetGroupInfo
+	revision string
+
+	// deletions is a list of previous versions of this object, that we should delete when asked to clean up.
+	deletions []fi.CloudupDeletion
+}
+
+// CreateNewRevisionsWith will create new revisions of the TargetGroup when the given networkLoadBalancer has a new revision.
+// This works around the fact that TargetGroups can only be attached to a single NetworkLoadBalancer.
+func (e *TargetGroup) CreateNewRevisionsWith(nlb *NetworkLoadBalancer) {
+	e.networkLoadBalancer = nlb
+}
+
+var _ fi.CloudupHasDependencies = &TargetGroup{}
+
+// GetDependencies returns the dependencies of the TargetGroup task
+// We need to do this because we hide the networkLoadBalancer field
+func (e *TargetGroup) GetDependencies(tasks map[string]fi.CloudupTask) []fi.CloudupTask {
+	var deps []fi.CloudupTask
+	deps = append(deps, e.VPC)
+	deps = append(deps, e.networkLoadBalancer)
+	return deps
 }
 
 var _ fi.CompareWithID = &TargetGroup{}
 
 func (e *TargetGroup) CompareWithID() *string {
+	if e.ARN != nil {
 		return e.ARN
 	}
+	return e.Name
+}
 
-func (e *TargetGroup) findTargetGroupByName(ctx context.Context, cloud awsup.AWSCloud) (*awsup.TargetGroupInfo, error) {
+func (e *TargetGroup) findLatestTargetGroupByName(ctx context.Context, cloud awsup.AWSCloud) (*awsup.TargetGroupInfo, error) {
 	name := fi.ValueOf(e.Name)
 
 	targetGroups, err := awsup.ListELBV2TargetGroups(ctx, cloud)
@@ -78,16 +109,57 @@ func (e *TargetGroup) findTargetGroupByName(ctx context.Context, cloud awsup.AWS
 	}
 
 	var latest *awsup.TargetGroupInfo
+	var latestRevision int
 	for _, targetGroup := range targetGroups {
 		// We accept the name tag _or_ the TargetGroupName itself, to allow matching groups that might predate tagging.
 		if aws.StringValue(targetGroup.TargetGroup.TargetGroupName) != name && targetGroup.NameTag() != name {
 			continue
 		}
-		if latest != nil {
+		revisionTag, _ := targetGroup.GetTag(awsup.KopsResourceRevisionTag)
-			return nil, fmt.Errorf("found multiple TargetGroups with name %q, expected 1", fi.ValueOf(e.Name))
+
+		revision := -1
+		if revisionTag == "" {
+			revision = 0
+		} else {
+			n, err := strconv.Atoi(revisionTag)
+			if err != nil {
+				klog.Warningf("ignoring target group %q with revision %q", targetGroup.ARN, revision)
+				continue
 			}
+			revision = n
+		}
+
+		if latest == nil || revision > latestRevision {
+			latestRevision = revision
 			latest = targetGroup
 		}
+	}
+
+	if latest != nil && e.networkLoadBalancer != nil {
+		matchRevision := e.networkLoadBalancer.revision
+		arn := e.networkLoadBalancer.loadBalancerArn
+		if arn == "" {
+			return nil, fmt.Errorf("load balancer not ready (no ARN)")
+		}
+		revisionTag, _ := latest.GetTag(awsup.KopsResourceRevisionTag)
+
+		if revisionTag != matchRevision {
+			klog.Warningf("found target group but revision %q does not match load balancer revision %q; will create a new target group", revisionTag, matchRevision)
+			latest = nil
+		}
+	}
+
+	// Record deletions for later
+	for _, targetGroup := range targetGroups {
+		if aws.StringValue(targetGroup.TargetGroup.TargetGroupName) != name && targetGroup.NameTag() != name {
+			continue
+		}
+		if latest != nil && latest.ARN == targetGroup.ARN {
+			continue
+		}
+
+		e.deletions = append(e.deletions, buildDeleteTargetGroup(targetGroup))
+	}
 
 	return latest, nil
 }
@@ -124,6 +196,7 @@ func (e *TargetGroup) findTargetGroupByARN(ctx context.Context, cloud awsup.AWSC
 
 	info := &awsup.TargetGroupInfo{
 		TargetGroup: tg,
+		ARN:         aws.StringValue(tg.TargetGroupArn),
 	}
 
 	for _, t := range tagResponse.TagDescriptions {
@@ -140,7 +213,7 @@ func (e *TargetGroup) Find(c *fi.CloudupContext) (*TargetGroup, error) {
 	var targetGroupInfo *awsup.TargetGroupInfo
 
 	if e.ARN == nil {
-		tgi, err := e.findTargetGroupByName(ctx, cloud)
+		tgi, err := e.findLatestTargetGroupByName(ctx, cloud)
 		if err != nil {
 			return nil, err
 		}
@@ -169,15 +242,23 @@ func (e *TargetGroup) Find(c *fi.CloudupContext) (*TargetGroup, error) {
 		UnhealthyThreshold: tg.UnhealthyThresholdCount,
 		VPC:                &VPC{ID: tg.VpcId},
 	}
+	actual.info = targetGroupInfo
+	e.info = targetGroupInfo
+	actual.revision, _ = targetGroupInfo.GetTag(awsup.KopsResourceRevisionTag)
+	e.revision = actual.revision
+
 	// Interval cannot be changed after TargetGroup creation
 	e.Interval = actual.Interval
 
 	e.ARN = tg.TargetGroupArn
-
 	tags := make(map[string]string)
 	for _, tag := range targetGroupInfo.Tags {
 		k := fi.ValueOf(tag.Key)
 		v := fi.ValueOf(tag.Value)
+		if k == awsup.KopsResourceRevisionTag {
+			actual.revision = v
+			continue
+		}
 		tags[k] = v
 	}
 	actual.Tags = tags
@@ -230,26 +311,58 @@ func (_ *TargetGroup) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *TargetGrou
 		return nil
 	}
 
+	tags := make(map[string]string)
+	for k, v := range e.Tags {
+		tags[k] = v
+	}
+	if a != nil {
+		if a.revision != "" {
+			tags[awsup.KopsResourceRevisionTag] = a.revision
+		}
+	}
+
+	if e.networkLoadBalancer != nil {
+		if e.networkLoadBalancer.loadBalancerArn == "" {
+			return fmt.Errorf("load balancer not yet ready (arn is empty)")
+		}
+		nlbRevision := e.networkLoadBalancer.revision
+		if nlbRevision != "" {
+			tags[awsup.KopsResourceRevisionTag] = nlbRevision
+		}
+	}
+
 	// You register targets for your Network Load Balancer with a target group. By default, the load balancer sends requests
 	// to registered targets using the port and protocol that you specified for the target group. You can override this port
 	// when you register each target with the target group.
 
 	if a == nil {
+		createTargetGroupName := *e.Name
+		if tags[awsup.KopsResourceRevisionTag] != "" {
+			s := *e.Name + tags[awsup.KopsResourceRevisionTag]
+			// We always compute the hash and add it, lest we trick users into assuming that we never do this
+			opt := truncate.TruncateStringOptions{
+				MaxLength:     32,
+				AlwaysAddHash: true,
+				HashLength:    6,
+			}
+			createTargetGroupName = truncate.TruncateString(s, opt)
+		}
+
 		request := &elbv2.CreateTargetGroupInput{
-			Name:                       e.Name,
+			Name:                       &createTargetGroupName,
 			Port:                       e.Port,
 			Protocol:                   e.Protocol,
 			VpcId:                      e.VPC.ID,
 			HealthCheckIntervalSeconds: e.Interval,
 			HealthyThresholdCount:      e.HealthyThreshold,
 			UnhealthyThresholdCount:    e.UnhealthyThreshold,
-			Tags:                       awsup.ELBv2Tags(e.Tags),
+			Tags:                       awsup.ELBv2Tags(tags),
 		}
 
 		klog.V(2).Infof("Creating Target Group for NLB")
 		response, err := t.Cloud.ELBV2().CreateTargetGroup(request)
 		if err != nil {
-			return fmt.Errorf("Error creating target group for NLB : %v", err)
+			return fmt.Errorf("creating NLB target group: %w", err)
 		}
 
 		if err := ModifyTargetGroupAttributes(t.Cloud, response.TargetGroups[0].TargetGroupArn, e.Attributes); err != nil {
@@ -259,6 +372,7 @@ func (_ *TargetGroup) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *TargetGrou
 		// Avoid spurious changes
 		e.ARN = response.TargetGroups[0].TargetGroupArn
 
+		// TODO: Set revision or info?
 	} else {
 		if a.ARN != nil {
 			if err := t.AddELBV2Tags(fi.ValueOf(a.ARN), e.Tags); err != nil {
@@ -364,3 +478,62 @@ func (e *TargetGroup) TerraformLink() *terraformWriter.Literal {
 	}
 	return terraformWriter.LiteralProperty("aws_lb_target_group", *e.Name, "id")
 }
+
+var _ fi.CloudupProducesDeletions = &TargetGroup{}
+
+// FindDeletions is responsible for finding launch templates which can be deleted
+func (e *TargetGroup) FindDeletions(c *fi.CloudupContext) ([]fi.CloudupDeletion, error) {
+	var removals []fi.CloudupDeletion
+	removals = append(removals, e.deletions...)
+	return removals, nil
+}
+
+// deleteTargetGroup tracks a TargetGroup that we're going to delete
+// It implements fi.CloudupDeletion
+type deleteTargetGroup struct {
+	obj *awsup.TargetGroupInfo
+}
+
+func buildDeleteTargetGroup(obj *awsup.TargetGroupInfo) *deleteTargetGroup {
+	d := &deleteTargetGroup{}
+	d.obj = obj
+	return d
+}
+
+var _ fi.CloudupDeletion = &deleteTargetGroup{}
+
+func (d *deleteTargetGroup) Delete(t fi.CloudupTarget) error {
+	ctx := context.TODO()
+
+	awsTarget, ok := t.(*awsup.AWSAPITarget)
+	if !ok {
+		return fmt.Errorf("unexpected target type for deletion: %T", t)
+	}
+
+	arn := d.obj.ARN
+	klog.V(2).Infof("deleting target group %q", arn)
+	if _, err := awsTarget.Cloud.ELBV2().DeleteTargetGroupWithContext(ctx, &elbv2.DeleteTargetGroupInput{
+		TargetGroupArn: &arn,
+	}); err != nil {
+		return fmt.Errorf("error deleting ELB TargetGroup %q: %w", arn, err)
+	}
+
+	return nil
+}
+
+// String returns a string representation of the task
+func (d *deleteTargetGroup) String() string {
+	return d.TaskName() + "-" + d.Item()
+}
+
+func (d *deleteTargetGroup) TaskName() string {
+	return "target-group"
+}
+
+func (d *deleteTargetGroup) Item() string {
+	return d.obj.ARN
+}
+
+func (d *deleteTargetGroup) DeferDeletion() bool {
+	return true
+}

upup/pkg/fi/cloudup/awstasks/vpc.go

@@ -391,3 +391,7 @@ func (d *deleteVPCCIDRBlock) TaskName() string {
 func (d *deleteVPCCIDRBlock) Item() string {
 	return fmt.Sprintf("%v: cidr=%v", *d.vpcID, *d.cidrBlock)
 }
+
+func (d *deleteVPCCIDRBlock) DeferDeletion() bool {
+	return false // TODO: should we defer this?
+}

upup/pkg/fi/cloudup/awsup/aws_cloud.go

@@ -164,8 +164,6 @@ type AWSCloud interface {
 	FindELBByNameTag(findNameTag string) (*elb.LoadBalancerDescription, error)
 	DescribeELBTags(loadBalancerNames []string) (map[string][]*elb.Tag, error)
 	// TODO: Remove, replace with awsup.ListELBV2LoadBalancers
-	FindELBV2ByNameTag(findNameTag string) (*elbv2.LoadBalancer, error)
-	// TODO: Remove, replace with awsup.ListELBV2LoadBalancers
 	DescribeELBV2Tags(loadBalancerNames []string) (map[string][]*elbv2.Tag, error)
 	FindELBV2NetworkInterfacesByName(vpcID string, loadBalancerName string) ([]*ec2.NetworkInterface, error)
 
@@ -1880,68 +1878,34 @@ func describeELBTags(c AWSCloud, loadBalancerNames []string) (map[string][]*elb.
 	return tagMap, nil
 }
 
-func (c *awsCloudImplementation) FindELBV2ByNameTag(findNameTag string) (*elbv2.LoadBalancer, error) {
+func FindLatestELBV2ByNameTag(loadBalancers []*LoadBalancerInfo, findNameTag string) *LoadBalancerInfo {
-	return findELBV2ByNameTag(c, findNameTag)
+	var latest *LoadBalancerInfo
-}
+	var latestRevision int
-
+	for _, lb := range loadBalancers {
-func findELBV2ByNameTag(c AWSCloud, findNameTag string) (*elbv2.LoadBalancer, error) {
+		if lb.NameTag() != findNameTag {
-	// TODO: Any way around this?
-	klog.V(2).Infof("Listing all NLBs for findNetworkLoadBalancerByNameTag")
-
-	request := &elbv2.DescribeLoadBalancersInput{}
-	// ELB DescribeTags has a limit of 20 names, so we set the page size here to 20 also
-	request.PageSize = aws.Int64(20)
-
-	var found []*elbv2.LoadBalancer
-
-	var innerError error
-	err := c.ELBV2().DescribeLoadBalancersPages(request, func(p *elbv2.DescribeLoadBalancersOutput, lastPage bool) bool {
-		if len(p.LoadBalancers) == 0 {
-			return true
-		}
-
-		// TODO: Filter by cluster?
-
-		var arns []string
-		arnToELB := make(map[string]*elbv2.LoadBalancer)
-		for _, elb := range p.LoadBalancers {
-			arn := aws.StringValue(elb.LoadBalancerArn)
-			arnToELB[arn] = elb
-			arns = append(arns, arn)
-		}
-
-		tagMap, err := c.DescribeELBV2Tags(arns)
-		if err != nil {
-			innerError = err
-			return false
-		}
-
-		for loadBalancerArn, tags := range tagMap {
-			name, foundNameTag := FindELBV2Tag(tags, "Name")
-			if !foundNameTag || name != findNameTag {
 			continue
 		}
-			elb := arnToELB[loadBalancerArn]
+		revisionTag, _ := lb.GetTag(KopsResourceRevisionTag)
-			found = append(found, elb)
+
-		}
+		revision := -1
-		return true
+		if revisionTag == "" {
-	})
+			revision = 0
+		} else {
+			n, err := strconv.Atoi(revisionTag)
 			if err != nil {
-		return nil, fmt.Errorf("error describing LoadBalancers: %v", err)
+				klog.Warningf("ignoring load balancer %q with revision %q", aws.StringValue(lb.LoadBalancer.LoadBalancerArn), revision)
+				continue
 			}
-	if innerError != nil {
+			revision = n
-		return nil, fmt.Errorf("error describing LoadBalancers: %v", innerError)
 		}
 
-	if len(found) == 0 {
+		if latest == nil || revision > latestRevision {
-		return nil, nil
+			latestRevision = revision
+			latest = lb
+		}
 	}
 
-	if len(found) != 1 {
+	return latest
-		return nil, fmt.Errorf("Found multiple NLBs with Name %q", findNameTag)
-	}
-
-	return found[0], nil
 }
 
 func (c *awsCloudImplementation) FindELBV2NetworkInterfacesByName(vpcID string, loadBalancerName string) ([]*ec2.NetworkInterface, error) {
@@ -2333,22 +2297,28 @@ func getApiIngressStatus(c AWSCloud, cluster *kops.Cluster) ([]fi.ApiIngressStat
 	return ingresses, nil
 }
 
-func findDNSName(c AWSCloud, cluster *kops.Cluster) (string, error) {
+func findDNSName(cloud AWSCloud, cluster *kops.Cluster) (string, error) {
+	ctx := context.TODO()
+
 	name := "api." + cluster.Name
 	if cluster.Spec.API.LoadBalancer == nil {
 		return "", nil
 	}
 	if cluster.Spec.API.LoadBalancer.Class == kops.LoadBalancerClassClassic {
-		if lb, err := c.FindELBByNameTag(name); err != nil {
+		if lb, err := cloud.FindELBByNameTag(name); err != nil {
 			return "", fmt.Errorf("error looking for AWS ELB: %v", err)
 		} else if lb != nil {
 			return aws.StringValue(lb.DNSName), nil
 		}
 	} else if cluster.Spec.API.LoadBalancer.Class == kops.LoadBalancerClassNetwork {
-		if lb, err := c.FindELBV2ByNameTag(name); err != nil {
+		allLoadBalancers, err := ListELBV2LoadBalancers(ctx, cloud)
-			return "", fmt.Errorf("error looking for AWS NLB: %v", err)
+		if err != nil {
-		} else if lb != nil {
+			return "", fmt.Errorf("looking for AWS NLB: %w", err)
-			return aws.StringValue(lb.DNSName), nil
+		}
+
+		latest := FindLatestELBV2ByNameTag(allLoadBalancers, name)
+		if latest != nil {
+			return aws.StringValue(latest.LoadBalancer.DNSName), nil
 		}
 	}
 	return "", nil

upup/pkg/fi/cloudup/awsup/aws_utils.go

@@ -241,8 +241,8 @@ func GetResourceName32(cluster string, prefix string) string {
 	return truncate.TruncateString(s, opt)
 }
 
-// GetTargetGroupNameFromARN will attempt to parse a target group ARN and return its name
+// NameForExternalTargetGroup will attempt to calculate a meaningful name for a target group given an ARN.
-func GetTargetGroupNameFromARN(targetGroupARN string) (string, error) {
+func NameForExternalTargetGroup(targetGroupARN string) (string, error) {
 	parsed, err := arn.Parse(targetGroupARN)
 	if err != nil {
 		return "", fmt.Errorf("error parsing target group ARN: %v", err)

upup/pkg/fi/cloudup/awsup/elbv2_targetgroups.go

@@ -29,12 +29,9 @@ import (
 type TargetGroupInfo struct {
 	TargetGroup *elbv2.TargetGroup
 	Tags        []*elbv2.Tag
-	arn         string
-}
 
-// ARN returns the ARN of the load balancer.
+	// ARN holds the arn (amazon id) of the target group.
-func (i *TargetGroupInfo) ARN() string {
+	ARN string
-	return i.arn
 }
 
 // NameTag returns the value of the tag with the key "Name".
@@ -72,7 +69,7 @@ func ListELBV2TargetGroups(ctx context.Context, cloud AWSCloud) ([]*TargetGroupI
 
 		for _, tg := range p.TargetGroups {
 			arn := aws.StringValue(tg.TargetGroupArn)
-			byARN[arn] = &TargetGroupInfo{TargetGroup: tg, arn: arn}
+			byARN[arn] = &TargetGroupInfo{TargetGroup: tg, ARN: arn}
 
 			tagRequest.ResourceArns = append(tagRequest.ResourceArns, tg.TargetGroupArn)
 		}

upup/pkg/fi/cloudup/awsup/mock_aws_cloud.go

@@ -211,10 +211,6 @@ func (c *MockAWSCloud) DescribeELBTags(loadBalancerNames []string) (map[string][
 	return describeELBTags(c, loadBalancerNames)
 }
 
-func (c *MockAWSCloud) FindELBV2ByNameTag(findNameTag string) (*elbv2.LoadBalancer, error) {
-	return findELBV2ByNameTag(c, findNameTag)
-}
-
 func (c *MockAWSCloud) DescribeELBV2Tags(loadBalancerArns []string) (map[string][]*elbv2.Tag, error) {
 	return describeELBV2Tags(c, loadBalancerArns)
 }

upup/pkg/fi/cloudup/awsup/tags.go

@@ -0,0 +1,24 @@
+/*
+Copyright 2024 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package awsup
+
+// KopsResourceRevisionTag is the tag used to store the revision timestamp,
+// when we are forced to create a new version of a resource because we cannot modify it in-place.
+// This happens when the resource field is immutable;
+// it also happens for ELBs, when we cannot have two ELBs pointing at the same target group
+// and thus must create a second.
+const KopsResourceRevisionTag = "kops.k8s.io/revision"

upup/pkg/fi/cloudup/openstacktasks/securitygroup.go

@@ -259,6 +259,10 @@ func (d *deleteSecurityGroup) Item() string {
 	return s
 }
 
+func (d *deleteSecurityGroup) DeferDeletion() bool {
+	return false // TODO: Should we defer deletion?
+}
+
 type deleteSecurityGroupRule struct {
 	rule          sgr.SecGroupRule
 	securityGroup *SecurityGroup
@@ -298,6 +302,10 @@ func (d *deleteSecurityGroupRule) Item() string {
 	return s
 }
 
+func (d *deleteSecurityGroupRule) DeferDeletion() bool {
+	return false // TODO: Should we defer deletion?
+}
+
 // RemovalRule is a rule that filters the permissions we should remove
 type RemovalRule interface {
 	Matches(sgr.SecGroupRule) bool

upup/pkg/fi/default_methods.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"reflect"
 
+	"k8s.io/klog/v2"
 	"k8s.io/kops/util/pkg/reflectutils"
 )
 
@@ -115,6 +116,17 @@ func defaultDeltaRunMethod[T SubContext](e Task[T], c *Context[T]) error {
 					return err
 				}
 			} else {
+				if deletion.DeferDeletion() {
+					switch c.deletionProcessingMode {
+					case DeletionProcessingModeDeleteIfNotDeferrred:
+						klog.Infof("not deleting %s/%s because it is marked for deferred-deletion", deletion.TaskName(), deletion.Item())
+						continue
+					case DeletionProcessingModeDeleteIncludingDeferred:
+						klog.V(2).Infof("processing deferred deletion of %s/%s", deletion.TaskName(), deletion.Item())
+					default:
+						klog.Fatalf("unhandled deletionProcessingMode %v", c.deletionProcessingMode)
+					}
+				}
 				if err := deletion.Delete(c.Target); err != nil {
 					return err
 				}

upup/pkg/fi/deletions.go

@@ -20,11 +20,13 @@ type DeletionProcessingMode string
 
 const (
 	// DeletionProcessingModeIgnore will ignore all deletion tasks.
+	// This is typically used when the target implements pruning directly (e.g. terraform)
 	DeletionProcessingModeIgnore DeletionProcessingMode = "Ignore"
-	// TODO: implement deferred-deletion in the tasks!
 	// DeletionProcessingModeDeleteIfNotDeferrred will delete resources only if they are not marked for deferred-deletion.
+	// This corresponds to a cluster update with --prune=false.
 	DeletionProcessingModeDeleteIfNotDeferrred DeletionProcessingMode = "IfNotDeferred"
 	// DeletionProcessingModeDeleteIncludingDeferrred will delete resources including those marked for deferred-deletion.
+	// This corresponds to a cluster update with --prune=true.
 	DeletionProcessingModeDeleteIncludingDeferred DeletionProcessingMode = "DeleteIncludingDeferred"
 )
 
@@ -38,6 +40,7 @@ type Deletion[T SubContext] interface {
 	Delete(target Target[T]) error
 	TaskName() string
 	Item() string
+	DeferDeletion() bool
 }
 
 type CloudupDeletion = Deletion[CloudupSubContext]

upup/pkg/fi/dryrun_target.go

@@ -256,10 +256,31 @@ func (t *DryRunTarget[T]) PrintReport(taskMap map[string]Task[T], out io.Writer)
 		// Give everything a consistent ordering
 		sort.Sort(DeletionByTaskName[T](t.deletions))
 
-		fmt.Fprintf(b, "Will delete items:\n")
+		var deferred []Deletion[T]
+		var immediate []Deletion[T]
+
 		for _, d := range t.deletions {
+			if d.DeferDeletion() {
+				deferred = append(deferred, d)
+			} else {
+				immediate = append(immediate, d)
+			}
+		}
+
+		if len(deferred) != 0 {
+			fmt.Fprintf(b, "Items will be deleted only when the --prune flag is specified:\n")
+			for _, d := range deferred {
 				fmt.Fprintf(b, "  %-20s %s\n", d.TaskName(), d.Item())
 			}
+			fmt.Fprintf(b, "\n")
+		}
+		if len(immediate) != 0 {
+			fmt.Fprintf(b, "Items will be deleted during update:\n")
+			for _, d := range immediate {
+				fmt.Fprintf(b, "  %-20s %s\n", d.TaskName(), d.Item())
+			}
+			fmt.Fprintf(b, "\n")
+		}
 	}
 
 	if len(t.assetBuilder.ImageAssets) != 0 {

upup/pkg/fi/topological_sort.go

@@ -66,9 +66,13 @@ func FindTaskDependencies[T SubContext](tasks map[string]Task[T]) map[string][]s
 
 		var dependencyKeys []string
 		for _, dep := range dependencies {
+			// Skip nils, including interface nils
+			if dep == nil || reflect.ValueOf(dep).IsNil() {
+				continue
+			}
 			dependencyKey, found := taskToId[dep]
 			if !found {
-				klog.Fatalf("dependency not found: %v", dep)
+				klog.Fatalf("dependency for task %T:%q not found: %v", t, k, dep)
 			}
 			dependencyKeys = append(dependencyKeys, dependencyKey)
 		}